diff --git a/CHANGELOG.md b/CHANGELOG.md index c7118bd..b3bf638 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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/`, 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 diff --git a/internal/cli/code.go b/internal/cli/code.go index de1298a..a831288 100644 --- a/internal/cli/code.go +++ b/internal/cli/code.go @@ -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() diff --git a/internal/cli/code_test.go b/internal/cli/code_test.go index 1abe3f5..0d7e2af 100644 --- a/internal/cli/code_test.go +++ b/internal/cli/code_test.go @@ -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") diff --git a/internal/cli/webvnc.go b/internal/cli/webvnc.go index dc8ad1c..ef71043 100644 --- a/internal/cli/webvnc.go +++ b/internal/cli/webvnc.go @@ -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() diff --git a/internal/cli/webvnc_test.go b/internal/cli/webvnc_test.go index 6c92907..c40a518 100644 --- a/internal/cli/webvnc_test.go +++ b/internal/cli/webvnc_test.go @@ -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", diff --git a/worker/src/fleet.ts b/worker/src/fleet.ts index ba14f53..3c8bbff 100644 --- a/worker/src/fleet.ts +++ b/worker/src/fleet.ts @@ -1356,7 +1356,7 @@ export class FleetDurableObject implements DurableObject { } private async consumeWebVNCTicket(request: Request): Promise { - 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 { - 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); } diff --git a/worker/test/fleet.test.ts b/worker/test/fleet.test.ts index 7ead113..c8d7389 100644 --- a/worker/test/fleet.test.ts +++ b/worker/test/fleet.test.ts @@ -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'",