Skip to content

refactor: harden Work service boundaries for Civilization #107

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
MichaelSaucier merged 1 commit into from
Sep 4, 2026
Merged

refactor: harden Work service boundaries for Civilization #107

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
16 changes: 0 additions & 16 deletions .tlc/adoption.json
View file
Open in desktop

This file was deleted.

25 changes: 6 additions & 19 deletions cmd/work-server/events.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"context"
"encoding/json"
"fmt"
"log"
"net/http"
"strings"
"sync"
Expand Down Expand Up @@ -123,25 +122,14 @@ func eventMatchesFilters(evType string, filters []string) bool {
return false
}

// --- SSE auth with query-string fallback ---
// --- SSE auth ---

// authSSE is the auth middleware for SSE endpoints. It accepts three sources,
// in order: Authorization: Bearer header, ws_key cookie, and `?key=` query
// parameter. The query-string arm exists because browser `EventSource` cannot
// set custom headers; callers that use it get a warning log (once per
// connection) recommending cookie auth.
// authSSE keeps credentials server-side by accepting only Authorization:
// Bearer. Browser EventSource clients connect to Site, whose proxy attaches the
// Work credential to its upstream request.
func (sv *server) authSSE(next http.HandlerFunc) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
if token, found := strings.CutPrefix(r.Header.Get("Authorization"), "Bearer "); found && token == sv.apiKey {
next(w, r)
return
}
if c, err := r.Cookie("ws_key"); err == nil && c.Value == sv.apiKey {
next(w, r)
return
}
if k := r.URL.Query().Get("key"); k != "" && k == sv.apiKey {
log.Printf("sse auth via query string from %s — key will appear in access logs, prefer cookie auth", r.RemoteAddr)
if token, found := strings.CutPrefix(r.Header.Get("Authorization"), "Bearer "); found && secureTokenEqual(token, sv.apiKey) {
next(w, r)
return
}
Expand All @@ -152,8 +140,7 @@ func (sv *server) authSSE(next http.HandlerFunc) http.HandlerFunc {
// --- /events/subscribe handler ---

// eventsSubscribe handles GET /events/subscribe[?types=<prefix1>,<prefix2>] —
// a raw, filtered SSE feed with no debounce. Accepts the same auth sources as
// /telemetry/sse.
// a raw, filtered SSE feed with no debounce.
func (sv *server) eventsSubscribe(w http.ResponseWriter, r *http.Request) {
flusher, ok := w.(http.Flusher)
if !ok {
Expand Down
46 changes: 13 additions & 33 deletions cmd/work-server/events_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ func sseClient(t *testing.T, ts *httptest.Server, path string) (<-chan string, c
if err != nil {
t.Fatalf("build request: %v", err)
}
req.Header.Set("Authorization", "Bearer test-key")
resp, err := ts.Client().Do(req)
if err != nil {
cancel()
Expand Down Expand Up @@ -136,7 +137,7 @@ func TestSSEKeepaliveComment(t *testing.T) {
// Verifying we get a 200 + correct Content-Type is enough: the keepalive
// ticker fires at 30s, longer than any reasonable unit-test budget.
ts, sv := newTestServer(t)
frames, cancel, resp := sseClient(t, ts, "/telemetry/sse?key=test-key")
frames, cancel, resp := sseClient(t, ts, "/telemetry/sse")
defer cancel()
if resp.StatusCode != 200 {
t.Fatalf("status: want 200, got %d", resp.StatusCode)
Expand All @@ -150,7 +151,7 @@ func TestSSEKeepaliveComment(t *testing.T) {

func TestSSEEventDelivery(t *testing.T) {
ts, sv := newTestServer(t)
frames, cancel, _ := sseClient(t, ts, "/telemetry/sse?key=test-key")
frames, cancel, _ := sseClient(t, ts, "/telemetry/sse")
defer cancel()
withSubscribers(t, sv, 1)

Expand All @@ -169,7 +170,7 @@ func TestSSEEventDelivery(t *testing.T) {

func TestSSEDebounce(t *testing.T) {
ts, sv := newTestServer(t)
frames, cancel, _ := sseClient(t, ts, "/telemetry/sse?key=test-key")
frames, cancel, _ := sseClient(t, ts, "/telemetry/sse")
defer cancel()
withSubscribers(t, sv, 1)

Expand All @@ -190,7 +191,7 @@ func TestSSEDebounce(t *testing.T) {

func TestSSEDisconnectCleanup(t *testing.T) {
ts, sv := newTestServer(t)
frames, cancel, _ := sseClient(t, ts, "/telemetry/sse?key=test-key")
frames, cancel, _ := sseClient(t, ts, "/telemetry/sse")
withSubscribers(t, sv, 1)

sv.fanout.Publish(mkEvent("hive.gap.detected", "cto", "hello"))
Expand All @@ -210,38 +211,17 @@ func TestSSEDisconnectCleanup(t *testing.T) {
t.Fatalf("fanout still has %d subscribers after disconnect", sv.fanout.NumSubscribers())
}

func TestSSEAuthQueryString(t *testing.T) {
func TestSSERejectsURLCredentials(t *testing.T) {
ts, _ := newTestServer(t)

// Valid key via ?key= — expect 200.
ok, err := ts.Client().Get(ts.URL + "/telemetry/sse?key=test-key")
if err != nil {
t.Fatalf("request: %v", err)
}
if ok.StatusCode != 200 {
t.Fatalf("valid key: want 200, got %d", ok.StatusCode)
}
ok.Body.Close()

// Invalid key — expect 401.
bad, err := ts.Client().Get(ts.URL + "/telemetry/sse?key=wrong")
resp, err := ts.Client().Get(ts.URL + "/telemetry/sse?key=test-key")
if err != nil {
t.Fatalf("request: %v", err)
}
if bad.StatusCode != 401 {
t.Fatalf("invalid key: want 401, got %d", bad.StatusCode)
}
bad.Body.Close()

// No auth at all — expect 401.
none, err := ts.Client().Get(ts.URL + "/telemetry/sse")
if err != nil {
t.Fatalf("request: %v", err)
}
if none.StatusCode != 401 {
t.Fatalf("no auth: want 401, got %d", none.StatusCode)
defer resp.Body.Close()
if resp.StatusCode != http.StatusUnauthorized {
t.Fatalf("query credential: want %d, got %d", http.StatusUnauthorized, resp.StatusCode)
}
none.Body.Close()
}

func TestSSEAuthBearer(t *testing.T) {
Expand All @@ -262,7 +242,7 @@ func TestSSEAuthBearer(t *testing.T) {

func TestEventsSubscribeNoFilter(t *testing.T) {
ts, sv := newTestServer(t)
frames, cancel, _ := sseClient(t, ts, "/events/subscribe?key=test-key")
frames, cancel, _ := sseClient(t, ts, "/events/subscribe")
defer cancel()
withSubscribers(t, sv, 1)

Expand All @@ -284,7 +264,7 @@ func TestEventsSubscribeNoFilter(t *testing.T) {

func TestEventsSubscribeMultiPrefix(t *testing.T) {
ts, sv := newTestServer(t)
frames, cancel, _ := sseClient(t, ts, "/events/subscribe?key=test-key&types=hive.*,site.op.*")
frames, cancel, _ := sseClient(t, ts, "/events/subscribe?types=hive.*,site.op.*")
defer cancel()
withSubscribers(t, sv, 1)

Expand All @@ -308,7 +288,7 @@ func TestEventsSubscribeMultiPrefix(t *testing.T) {
func TestEventsSubscribeEmptyTypes(t *testing.T) {
ts, sv := newTestServer(t)
// Empty ?types= value should behave exactly like no filter.
frames, cancel, _ := sseClient(t, ts, "/events/subscribe?key=test-key&types=")
frames, cancel, _ := sseClient(t, ts, "/events/subscribe?types=")
defer cancel()
withSubscribers(t, sv, 1)

Expand Down
41 changes: 19 additions & 22 deletions cmd/work-server/legacy_ui_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
"testing"
)

func TestLegacyBrowserUIHeadersAndNotice(t *testing.T) {
func TestLegacyBrowserUICannotBeReenabledOrExposeCredentials(t *testing.T) {
t.Setenv("WORK_LEGACY_BROWSER_UI", "1")
sv := &server{apiKey: "test-key", apiToken: "test-token"}
cases := []struct {
Expand Down Expand Up @@ -40,18 +40,17 @@ func TestLegacyBrowserUIHeadersAndNotice(t *testing.T) {
rec := httptest.NewRecorder()
tc.handler(rec, tc.req)
resp := rec.Result()
if got := resp.Header.Get(legacyUIStatusHeader); got != "legacy" {
t.Fatalf("%s = %q, want legacy", legacyUIStatusHeader, got)
if resp.StatusCode != http.StatusFound {
t.Fatalf("status = %d, want %d", resp.StatusCode, http.StatusFound)
}
if got := resp.Header.Get(legacyUIStatusHeader); got != "disabled" {
t.Fatalf("%s = %q, want disabled", legacyUIStatusHeader, got)
}
if got := resp.Header.Get(legacyUIReplacementHeader); got != tc.wantReplacement {
t.Fatalf("%s = %q, want %q", legacyUIReplacementHeader, got, tc.wantReplacement)
}
body := rec.Body.String()
if !strings.Contains(body, "Legacy Work browser UI") {
t.Fatalf("body missing legacy notice")
}
if !strings.Contains(body, tc.wantReplacement) {
t.Fatalf("body missing replacement URL %q", tc.wantReplacement)
if strings.Contains(rec.Body.String(), "test-key") || strings.Contains(rec.Body.String(), "test-token") || len(resp.Cookies()) != 0 {
t.Fatal("retired UI exposed a credential")
}
})
}
Expand Down Expand Up @@ -119,19 +118,17 @@ func TestAPIRoutesDoNotCarryLegacyUIHeaders(t *testing.T) {
}
}

func TestLegacyBrowserUIEnabled(t *testing.T) {
for _, value := range []string{"1", "true", "TRUE", "yes", "on"} {
t.Run(value, func(t *testing.T) {
t.Setenv("WORK_LEGACY_BROWSER_UI", value)
if !legacyBrowserUIEnabled() {
t.Fatal("legacyBrowserUIEnabled() = false, want true")
}
})
}

t.Setenv("WORK_LEGACY_BROWSER_UI", "")
if legacyBrowserUIEnabled() {
t.Fatal("legacyBrowserUIEnabled() = true, want false")
func TestAPIRejectsRetiredCookieCredential(t *testing.T) {
sv := &server{apiKey: "test-key"}
handler := sv.auth(func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusNoContent)
})
req := httptest.NewRequest(http.MethodGet, "http://nucbuntu:8080/tasks", nil)
req.AddCookie(&http.Cookie{Name: "ws_key", Value: "test-key"})
rec := httptest.NewRecorder()
handler(rec, req)
if rec.Code != http.StatusUnauthorized {
t.Fatalf("cookie credential status = %d, want %d", rec.Code, http.StatusUnauthorized)
}
}

Expand Down
Loading
Loading