fix: keep bridge tickets out of websocket urls
This commit is contained in:
parent
120802c150
commit
1fc07c4efa
@ -8,6 +8,7 @@
|
||||
- Fixed `run --no-sync` timing summaries so they report `sync_skipped=true`.
|
||||
- Fixed native Windows command output so first-use PowerShell progress records do not leak CLIXML into run logs.
|
||||
- Fixed Islo provider sync so `crabbox run --provider islo` uploads the local workspace, uses the correct `/workspace/<workdir>`, and falls back to chunked exec upload while the archive API returns server errors.
|
||||
- Fixed Code and WebVNC bridge websocket auth so upgraded brokers receive short-lived bridge tickets in the `Authorization` header instead of logging them in URL query strings, while preserving query fallback for older brokers.
|
||||
|
||||
## 0.6.0 - 2026-05-07
|
||||
|
||||
|
||||
@ -250,9 +250,14 @@ func connectCodeBridge(ctx context.Context, coord *CoordinatorClient, leaseID, h
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
ws, _, err := websocket.Dial(ctx, webCodeAgentURL(coord.BaseURL, leaseID, ticket.Ticket), &websocket.DialOptions{
|
||||
HTTPHeader: coord.webVNCAccessHeaders(),
|
||||
ws, resp, err := websocket.Dial(ctx, webCodeAgentURL(coord.BaseURL, leaseID), &websocket.DialOptions{
|
||||
HTTPHeader: bridgeTicketHeaders(coord, ticket.Ticket),
|
||||
})
|
||||
if retryBridgeTicketInQuery(resp, err) {
|
||||
ws, _, err = websocket.Dial(ctx, webCodeAgentURLWithTicket(coord.BaseURL, leaseID, ticket.Ticket), &websocket.DialOptions{
|
||||
HTTPHeader: coord.webVNCAccessHeaders(),
|
||||
})
|
||||
}
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
@ -690,7 +695,7 @@ func availableLocalCodePort() string {
|
||||
return "8081"
|
||||
}
|
||||
|
||||
func webCodeAgentURL(base, leaseID, ticket string) string {
|
||||
func webCodeAgentURL(base, leaseID string) string {
|
||||
u, err := url.Parse(base)
|
||||
if err != nil {
|
||||
return base
|
||||
@ -701,6 +706,16 @@ func webCodeAgentURL(base, leaseID, ticket string) string {
|
||||
u.Scheme = "ws"
|
||||
}
|
||||
u.Path = strings.TrimRight(u.Path, "/") + "/v1/leases/" + url.PathEscape(leaseID) + "/code/agent"
|
||||
u.RawQuery = ""
|
||||
u.Fragment = ""
|
||||
return u.String()
|
||||
}
|
||||
|
||||
func webCodeAgentURLWithTicket(base, leaseID, ticket string) string {
|
||||
u, err := url.Parse(webCodeAgentURL(base, leaseID))
|
||||
if err != nil {
|
||||
return base
|
||||
}
|
||||
values := url.Values{}
|
||||
values.Set("ticket", ticket)
|
||||
u.RawQuery = values.Encode()
|
||||
|
||||
@ -1,20 +1,27 @@
|
||||
package cli
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/base64"
|
||||
"encoding/json"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"nhooyr.io/websocket"
|
||||
)
|
||||
|
||||
func TestWebCodeURLs(t *testing.T) {
|
||||
if got := webCodeAgentURL("https://crabbox.openclaw.ai", "cbx_abcdef123456", "code_abc"); got != "wss://crabbox.openclaw.ai/v1/leases/cbx_abcdef123456/code/agent?ticket=code_abc" {
|
||||
if got := webCodeAgentURL("https://crabbox.openclaw.ai", "cbx_abcdef123456"); got != "wss://crabbox.openclaw.ai/v1/leases/cbx_abcdef123456/code/agent" {
|
||||
t.Fatalf("agent URL=%q", got)
|
||||
}
|
||||
if got := webCodeAgentURLWithTicket("https://crabbox.openclaw.ai", "cbx_abcdef123456", "code_abc"); got != "wss://crabbox.openclaw.ai/v1/leases/cbx_abcdef123456/code/agent?ticket=code_abc" {
|
||||
t.Fatalf("agent fallback URL=%q", got)
|
||||
}
|
||||
if got := webCodePortalURL("https://crabbox.openclaw.ai/", "cbx_abcdef123456"); got != "https://crabbox.openclaw.ai/portal/leases/cbx_abcdef123456/code/" {
|
||||
t.Fatalf("portal URL=%q", got)
|
||||
}
|
||||
@ -23,6 +30,58 @@ func TestWebCodeURLs(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestConnectCodeBridgeSendsTicketInAuthorizationHeader(t *testing.T) {
|
||||
agentConnected := make(chan struct{})
|
||||
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
switch r.URL.Path {
|
||||
case "/v1/leases/cbx_abcdef123456/code/ticket":
|
||||
if r.Method != http.MethodPost {
|
||||
t.Errorf("ticket method=%s", r.Method)
|
||||
}
|
||||
if got := r.Header.Get("Authorization"); got != "Bearer test-token" {
|
||||
t.Errorf("authorization=%q", got)
|
||||
}
|
||||
_ = json.NewEncoder(w).Encode(coordinatorCodeTicket{
|
||||
Ticket: "code_abcdef1234567890abcdef1234567890",
|
||||
LeaseID: "cbx_abcdef123456",
|
||||
})
|
||||
case "/v1/leases/cbx_abcdef123456/code/agent":
|
||||
if got := r.URL.Query().Get("ticket"); got != "" {
|
||||
t.Errorf("query ticket=%q", got)
|
||||
}
|
||||
if got := r.Header.Get("Authorization"); got != "Bearer code_abcdef1234567890abcdef1234567890" {
|
||||
t.Errorf("bridge authorization=%q", got)
|
||||
}
|
||||
conn, err := websocket.Accept(w, r, nil)
|
||||
if err != nil {
|
||||
t.Errorf("websocket accept: %v", err)
|
||||
return
|
||||
}
|
||||
close(agentConnected)
|
||||
_, _, _ = conn.Read(context.Background())
|
||||
_ = conn.Close(websocket.StatusNormalClosure, "test done")
|
||||
default:
|
||||
http.NotFound(w, r)
|
||||
}
|
||||
}))
|
||||
defer server.Close()
|
||||
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
|
||||
defer cancel()
|
||||
coord := &CoordinatorClient{BaseURL: server.URL, Token: "test-token", Client: server.Client()}
|
||||
bridge, err := connectCodeBridge(ctx, coord, "cbx_abcdef123456", "127.0.0.1", "8080")
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
defer bridge.Close(websocket.StatusNormalClosure, "test done")
|
||||
|
||||
select {
|
||||
case <-agentConnected:
|
||||
case <-ctx.Done():
|
||||
t.Fatal(ctx.Err())
|
||||
}
|
||||
}
|
||||
|
||||
func TestMappedRemoteCodeFolderTracksCurrentSubdirectory(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
subdir := filepath.Join(root, "worker", "src")
|
||||
|
||||
@ -398,9 +398,14 @@ func connectWebVNCBridge(ctx context.Context, coord *CoordinatorClient, leaseID,
|
||||
_ = tcp.Close()
|
||||
return nil, err
|
||||
}
|
||||
ws, _, err := websocket.Dial(ctx, webVNCAgentURL(coord.BaseURL, leaseID, ticket.Ticket), &websocket.DialOptions{
|
||||
HTTPHeader: coord.webVNCAccessHeaders(),
|
||||
ws, resp, err := websocket.Dial(ctx, webVNCAgentURL(coord.BaseURL, leaseID), &websocket.DialOptions{
|
||||
HTTPHeader: bridgeTicketHeaders(coord, ticket.Ticket),
|
||||
})
|
||||
if retryBridgeTicketInQuery(resp, err) {
|
||||
ws, _, err = websocket.Dial(ctx, webVNCAgentURLWithTicket(coord.BaseURL, leaseID, ticket.Ticket), &websocket.DialOptions{
|
||||
HTTPHeader: coord.webVNCAccessHeaders(),
|
||||
})
|
||||
}
|
||||
if err != nil {
|
||||
_ = tcp.Close()
|
||||
return nil, err
|
||||
@ -506,7 +511,24 @@ func (c *CoordinatorClient) webVNCAccessHeaders() http.Header {
|
||||
return header
|
||||
}
|
||||
|
||||
func webVNCAgentURL(base, leaseID, ticket string) string {
|
||||
func bridgeTicketHeaders(coord *CoordinatorClient, ticket string) http.Header {
|
||||
headers := coord.webVNCAccessHeaders()
|
||||
headers.Set("Authorization", "Bearer "+ticket)
|
||||
return headers
|
||||
}
|
||||
|
||||
func retryBridgeTicketInQuery(resp *http.Response, err error) bool {
|
||||
if err == nil || resp == nil {
|
||||
return false
|
||||
}
|
||||
if resp.Body != nil {
|
||||
_, _ = io.Copy(io.Discard, resp.Body)
|
||||
_ = resp.Body.Close()
|
||||
}
|
||||
return resp.StatusCode == http.StatusUnauthorized
|
||||
}
|
||||
|
||||
func webVNCAgentURL(base, leaseID string) string {
|
||||
u, err := url.Parse(base)
|
||||
if err != nil {
|
||||
return base
|
||||
@ -517,6 +539,16 @@ func webVNCAgentURL(base, leaseID, ticket string) string {
|
||||
u.Scheme = "ws"
|
||||
}
|
||||
u.Path = strings.TrimRight(u.Path, "/") + "/v1/leases/" + url.PathEscape(leaseID) + "/webvnc/agent"
|
||||
u.RawQuery = ""
|
||||
u.Fragment = ""
|
||||
return u.String()
|
||||
}
|
||||
|
||||
func webVNCAgentURLWithTicket(base, leaseID, ticket string) string {
|
||||
u, err := url.Parse(webVNCAgentURL(base, leaseID))
|
||||
if err != nil {
|
||||
return base
|
||||
}
|
||||
values := url.Values{}
|
||||
values.Set("ticket", ticket)
|
||||
u.RawQuery = values.Encode()
|
||||
|
||||
@ -4,6 +4,7 @@ import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"io"
|
||||
"net"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
@ -18,9 +19,12 @@ import (
|
||||
)
|
||||
|
||||
func TestWebVNCURLs(t *testing.T) {
|
||||
if got := webVNCAgentURL("https://crabbox.openclaw.ai", "cbx_abcdef123456", "wvnc_abc"); got != "wss://crabbox.openclaw.ai/v1/leases/cbx_abcdef123456/webvnc/agent?ticket=wvnc_abc" {
|
||||
if got := webVNCAgentURL("https://crabbox.openclaw.ai", "cbx_abcdef123456"); got != "wss://crabbox.openclaw.ai/v1/leases/cbx_abcdef123456/webvnc/agent" {
|
||||
t.Fatalf("agent URL=%q", got)
|
||||
}
|
||||
if got := webVNCAgentURLWithTicket("https://crabbox.openclaw.ai", "cbx_abcdef123456", "wvnc_abc"); got != "wss://crabbox.openclaw.ai/v1/leases/cbx_abcdef123456/webvnc/agent?ticket=wvnc_abc" {
|
||||
t.Fatalf("agent fallback URL=%q", got)
|
||||
}
|
||||
if got := webVNCPortalURL("https://crabbox.openclaw.ai/", "cbx_abcdef123456", "", "secret value"); got != "https://crabbox.openclaw.ai/portal/leases/cbx_abcdef123456/vnc#password=secret+value" {
|
||||
t.Fatalf("portal URL=%q", got)
|
||||
}
|
||||
@ -78,8 +82,11 @@ func TestConnectWebVNCBridgeRegistersAgentBeforeServe(t *testing.T) {
|
||||
LeaseID: "cbx_abcdef123456",
|
||||
})
|
||||
case "/v1/leases/cbx_abcdef123456/webvnc/agent":
|
||||
if got := r.URL.Query().Get("ticket"); got != "wvnc_abcdef1234567890abcdef1234567890" {
|
||||
t.Errorf("ticket=%q", got)
|
||||
if got := r.URL.Query().Get("ticket"); got != "" {
|
||||
t.Errorf("query ticket=%q", got)
|
||||
}
|
||||
if got := r.Header.Get("Authorization"); got != "Bearer wvnc_abcdef1234567890abcdef1234567890" {
|
||||
t.Errorf("bridge authorization=%q", got)
|
||||
}
|
||||
conn, err := websocket.Accept(w, r, nil)
|
||||
if err != nil {
|
||||
@ -156,6 +163,22 @@ func TestRetryableWebVNCBridgeErrors(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestRetryBridgeTicketInQuery(t *testing.T) {
|
||||
resp := &http.Response{
|
||||
StatusCode: http.StatusUnauthorized,
|
||||
Body: io.NopCloser(strings.NewReader("old broker needs query ticket")),
|
||||
}
|
||||
if !retryBridgeTicketInQuery(resp, errors.New("websocket rejected")) {
|
||||
t.Fatal("expected unauthorized websocket response to retry with query ticket")
|
||||
}
|
||||
if retryBridgeTicketInQuery(&http.Response{StatusCode: http.StatusForbidden}, errors.New("forbidden")) {
|
||||
t.Fatal("forbidden response should not retry with query ticket")
|
||||
}
|
||||
if retryBridgeTicketInQuery(resp, nil) {
|
||||
t.Fatal("successful dial should not retry with query ticket")
|
||||
}
|
||||
}
|
||||
|
||||
func TestWebVNCDaemonArgsStripBackgroundFlags(t *testing.T) {
|
||||
got := strings.Join(stripWebVNCDaemonFlags([]string{
|
||||
"--provider",
|
||||
|
||||
@ -1356,7 +1356,7 @@ export class FleetDurableObject implements DurableObject {
|
||||
}
|
||||
|
||||
private async consumeWebVNCTicket(request: Request): Promise<WebVNCTicketRecord | undefined> {
|
||||
const value = new URL(request.url).searchParams.get("ticket") ?? "";
|
||||
const value = bridgeTicketFromRequest(request);
|
||||
if (!validWebVNCTicket(value)) {
|
||||
return undefined;
|
||||
}
|
||||
@ -1385,7 +1385,7 @@ export class FleetDurableObject implements DurableObject {
|
||||
}
|
||||
|
||||
private async consumeCodeTicket(request: Request): Promise<CodeTicketRecord | undefined> {
|
||||
const value = new URL(request.url).searchParams.get("ticket") ?? "";
|
||||
const value = bridgeTicketFromRequest(request);
|
||||
if (!validCodeTicket(value)) {
|
||||
return undefined;
|
||||
}
|
||||
@ -2173,6 +2173,15 @@ function validCodeTicket(value: string | undefined): value is string {
|
||||
return typeof value === "string" && /^code_[a-f0-9]{32}$/.test(value);
|
||||
}
|
||||
|
||||
export function bridgeTicketFromRequest(request: Request): string {
|
||||
const auth = request.headers.get("authorization")?.trim() ?? "";
|
||||
const match = /^Bearer\s+(.+)$/i.exec(auth);
|
||||
if (match?.[1]) {
|
||||
return match[1].trim();
|
||||
}
|
||||
return new URL(request.url).searchParams.get("ticket") ?? "";
|
||||
}
|
||||
|
||||
function validImageID(value: string | undefined): value is string {
|
||||
return typeof value === "string" && /^ami-[a-f0-9]{8,32}$/.test(value);
|
||||
}
|
||||
|
||||
@ -2,6 +2,7 @@ import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
import {
|
||||
FleetDurableObject,
|
||||
bridgeTicketFromRequest,
|
||||
codeForwardHeaders,
|
||||
codeResponseHeaders,
|
||||
flushPendingWebVNC,
|
||||
@ -1318,6 +1319,21 @@ describe("fleet lease identity and idle", () => {
|
||||
expect(missingTicket.status).toBe(401);
|
||||
});
|
||||
|
||||
it("accepts bridge tickets in authorization before falling back to query strings", () => {
|
||||
expect(
|
||||
bridgeTicketFromRequest(
|
||||
request("GET", "/v1/leases/blue-lobster/code/agent?ticket=code_query", {
|
||||
headers: { authorization: "Bearer code_header" },
|
||||
}),
|
||||
),
|
||||
).toBe("code_header");
|
||||
expect(
|
||||
bridgeTicketFromRequest(
|
||||
request("GET", "/v1/leases/blue-lobster/code/agent?ticket=code_query"),
|
||||
),
|
||||
).toBe("code_query");
|
||||
});
|
||||
|
||||
it("uses a VS Code-compatible CSP for code proxy responses", () => {
|
||||
const headers = codeResponseHeaders({
|
||||
"content-security-policy": "default-src 'none'; script-src 'self'",
|
||||
|
||||
Loading…
Reference in New Issue
Block a user