diff --git a/desktop/app.go b/desktop/app.go index a46cb53..8c0cb9c 100644 --- a/desktop/app.go +++ b/desktop/app.go @@ -43,6 +43,9 @@ type App struct { broker *agentmqtt.Client stopBridge func() hookURL string + // Relays camera feeds to the webview so the engine's credential never has + // to travel in an src, which a Chromium webview would strip anyway. + proxy *streamProxy // Set once the operator logs in. Until then the UI shows the login sheet // and nothing else is reachable. onSessionChange func(bool) @@ -63,6 +66,7 @@ func NewApp() *App { cfg: cfg, cloud: cloud.New(envOr("BEHAVISION_CLOUD", "https://mcp.loyaly.ai")), local: local.New(base, cfg.APIUser, cfg.APIPassword), + proxy: newStreamProxy(), } } @@ -70,6 +74,14 @@ func (a *App) startup(ctx context.Context) { a.ctx = ctx _ = agentpaths.EnsureState() + // Before any screen asks for a camera URL. A failure here is logged and + // not fatal: the rest of the app - people, cameras, the engine controls - + // works without a picture, and refusing to start over a broken tile would + // take a working shop offline. + if err := a.proxy.start(a.local.Base, a.local.User, a.local.Password); err != nil { + log.Printf("camera relay unavailable, tiles will not load: %v", err) + } + // A saved session means a shop PC that rebooted overnight comes back // working instead of waiting for someone to log in. if a.cfg.SessionToken != "" { @@ -273,8 +285,8 @@ type PipelineStatus struct { // Standalone separates "nothing is being sent because this PC is set up on // its own" from "nothing is being sent and something is wrong". They look // identical from the counters alone, and only one of them is a fault. - Standalone bool `json:"standalone"` - BrokerUp bool `json:"broker_up"` + Standalone bool `json:"standalone"` + BrokerUp bool `json:"broker_up"` Accepted uint64 `json:"accepted"` } @@ -549,15 +561,25 @@ func (a *App) PlacementResult(id string) (map[string]any, error) { return a.local.PlacementResult(ctx, id) } -// StreamURL is the MJPEG endpoint for a camera, with credentials inline so an -// tag can load it. Loopback only - it never leaves this machine. +// StreamURL is the MJPEG endpoint for a camera tile. +// +// It points at this app's own loopback relay, not at the engine directly. The +// previous version put the engine's Basic credentials inline in the URL, with +// a comment saying they were there "so an tag can load it" - which a +// browser will not do. Chromium strips credentials from subresource URLs, and +// WebView2 is Chromium, so every camera tile on a shop PC was a broken image. +// See stream_proxy.go for the measurement. +// +// The relay is also why no password appears in the page any more. If it is not +// running the fallback is the bare engine URL with no credential: correct for +// an engine configured without auth, and for one with auth a tile that fails +// to load rather than a password sitting in the DOM. func (a *App) StreamURL(cameraID string) string { - base := strings.TrimPrefix(strings.TrimPrefix(a.local.Base, "http://"), "https://") - if a.local.User == "" { - return fmt.Sprintf("http://%s/api/cameras/%s/stream.mjpeg", base, cameraID) + if u := a.proxy.urlFor(cameraID, "stream.mjpeg"); u != "" { + return u } - return fmt.Sprintf("http://%s:%s@%s/api/cameras/%s/stream.mjpeg", - a.local.User, a.local.Password, base, cameraID) + base := strings.TrimPrefix(strings.TrimPrefix(a.local.Base, "http://"), "https://") + return fmt.Sprintf("http://%s/api/cameras/%s/stream.mjpeg", base, cameraID) } // ------------------------------------------------------------------- live -- diff --git a/desktop/main.go b/desktop/main.go index 83dc2a7..3f56748 100644 --- a/desktop/main.go +++ b/desktop/main.go @@ -49,6 +49,7 @@ func main() { }, OnShutdown: func(ctx context.Context) { tray.stop() + app.proxy.stop() app.StopEngine() }, Bind: []any{app}, diff --git a/desktop/stream_proxy.go b/desktop/stream_proxy.go new file mode 100644 index 0000000..6d57c84 --- /dev/null +++ b/desktop/stream_proxy.go @@ -0,0 +1,249 @@ +package main + +// streamProxy serves the engine's camera feeds to this app's own webview +// without putting a credential in the page. +// +// What this replaces: StreamURL used to build +// http://user:pass@127.0.0.1:8010/api/cameras//stream.mjpeg and hand it +// to an , with a comment saying the credentials were inline "so an +// tag can load it". It cannot. Chromium strips credentials from subresource +// URLs and has since M59, and WebView2 is Chromium - so on the one platform +// this product ships to, every camera tile on the shop floor renders as a +// broken image. Measured against the same running engine: the app's Go-side +// calls returned stats and people while an on the very same URL failed, +// and curl proved the URL itself answered 200. The engine was never the +// problem; the browser was throwing the password away before it asked. +// +// So the password stays on this side of the process boundary. The webview +// asks this loopback listener, the listener attaches Basic auth and relays +// the engine's bytes back unchanged. It is the same reasoning the head-office +// web app already follows in Shot.jsx, where an equally cannot carry a +// session and the bytes are fetched and handed over as an object URL. + +import ( + "crypto/rand" + "crypto/subtle" + "encoding/hex" + "fmt" + "net" + "net/http" + "net/url" + "regexp" + "strings" + "sync" + "time" +) + +// A camera id reaches this from the engine and from a person typing into the +// Add Camera form. Validated rather than interpolated: without this a `..` +// would climb out of the two paths below and turn a camera relay into a proxy +// for any engine endpoint, with the credential helpfully attached. +var safeCameraIDChars = regexp.MustCompile(`^[A-Za-z0-9_.-]{1,64}$`) + +// safeCameraID is the character check AND the two names that pass it and still +// mean something to a path resolver. +// +// The pattern allows `.` because real camera ids contain them - which means it +// also allows exactly `.` and `..`, and `/api/cameras/../stream.mjpeg` is not +// the endpoint anyone intended. The id can never contain a slash (the path is +// split on them before we get here), so these two strings are the entire +// remaining traversal surface. Found by the test, not by reading the regex. +func safeCameraID(id string) bool { + if id == "." || id == ".." { + return false + } + return safeCameraIDChars.MatchString(id) +} + +type streamProxy struct { + mu sync.RWMutex + ln net.Listener + srv *http.Server + client *http.Client + token string + target string // engine origin, e.g. http://127.0.0.1:8010 + user string + pass string +} + +func newStreamProxy() *streamProxy { return &streamProxy{} } + +// start binds a loopback listener and begins relaying. Calling it again while +// running is a no-op, so a restarted engine cannot leave two listeners behind. +func (p *streamProxy) start(base, user, pass string) error { + p.mu.Lock() + defer p.mu.Unlock() + if p.srv != nil { + return nil + } + + if !strings.HasPrefix(base, "http://") && !strings.HasPrefix(base, "https://") { + base = "http://" + base + } + if _, err := url.Parse(base); err != nil { + return fmt.Errorf("engine base %q: %w", base, err) + } + + // The engine's own credential exists precisely so that the live face feed + // is never served open - CLAUDE.md is explicit that an unauthenticated + // listener would expose it. An unauthenticated loopback relay would hand + // that same feed to any other process on this PC, which on a shop counter + // is not a theoretical set. A per-run token, minted here and given only to + // this app's own webview, keeps the relay as private as the engine is. + raw := make([]byte, 32) + if _, err := rand.Read(raw); err != nil { + return fmt.Errorf("proxy token: %w", err) + } + + // Port 0: the OS picks a free one. A fixed port would collide with + // whatever else a shop PC happens to be running, and the failure would be + // "the cameras stopped working" with nothing pointing at the cause. + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + return fmt.Errorf("stream proxy listen: %w", err) + } + + p.ln = ln + p.token = hex.EncodeToString(raw) + p.target = strings.TrimRight(base, "/") + p.user, p.pass = user, pass + // No client timeout: an MJPEG stream is endless by design and any deadline + // would cut the picture off mid-shift. The request context ends it when + // the webview navigates away or the tile is replaced. + p.client = &http.Client{ + Transport: &http.Transport{ + DialContext: (&net.Dialer{Timeout: 5 * time.Second}).DialContext, + TLSHandshakeTimeout: 5 * time.Second, + }, + } + srv := &http.Server{Handler: http.HandlerFunc(p.handle)} + p.srv = srv + + // srv and ln are captured, not read off the struct inside the goroutine: + // stop() sets both to nil, so a serve loop that reached for them after a + // quick start/stop would dereference nil and take the whole app down. The + // test that stops the relay found exactly that. + go func() { _ = srv.Serve(ln) }() + return nil +} + +func (p *streamProxy) stop() { + p.mu.Lock() + srv, ln := p.srv, p.ln + p.srv, p.ln, p.token = nil, nil, "" + p.mu.Unlock() + if srv != nil { + _ = srv.Close() + } + if ln != nil { + _ = ln.Close() + } +} + +// urlFor returns the loopback URL for one camera resource, or "" when the +// proxy is not running so the caller can fall back. +func (p *streamProxy) urlFor(cameraID, file string) string { + p.mu.RLock() + defer p.mu.RUnlock() + if p.ln == nil || p.token == "" || !safeCameraID(cameraID) { + return "" + } + return fmt.Sprintf("http://%s/s/%s/%s/%s", + p.ln.Addr().String(), p.token, cameraID, file) +} + +func (p *streamProxy) handle(w http.ResponseWriter, r *http.Request) { + p.mu.RLock() + token, target, user, pass, client := p.token, p.target, p.user, p.pass, p.client + p.mu.RUnlock() + if token == "" || client == nil { + http.NotFound(w, r) + return + } + + // /s/// + parts := strings.Split(strings.TrimPrefix(r.URL.Path, "/"), "/") + if len(parts) != 4 || parts[0] != "s" { + http.NotFound(w, r) + return + } + // Constant time: the token is the only thing standing between another + // local process and a live view of customers' faces. + if subtle.ConstantTimeCompare([]byte(parts[1]), []byte(token)) != 1 { + // 404 rather than 403. There is nothing here to tell an unwelcome + // caller they have found the right door with the wrong key. + http.NotFound(w, r) + return + } + cameraID := parts[2] + if !safeCameraID(cameraID) { + http.NotFound(w, r) + return + } + + // An allow-list, not a prefix match. Everything else the engine serves - + // the identity list, the gallery, erasure - stays unreachable through here + // even for a caller holding the token. + // + // frame.jpg is listed although no screen asks for one yet. It is reachable + // only through urlFor, which is internal, so it adds no bound API nobody + // calls; it is here so that adding a still later is a change to a screen + // rather than a change to the one file where a mistake is a credentialed + // proxy onto the biometric API. + var enginePath string + switch parts[3] { + case "stream.mjpeg": + enginePath = "/api/cameras/" + cameraID + "/stream.mjpeg" + case "frame.jpg": + enginePath = "/api/cameras/" + cameraID + "/frame.jpg" + default: + http.NotFound(w, r) + return + } + + req, err := http.NewRequestWithContext(r.Context(), http.MethodGet, target+enginePath, nil) + if err != nil { + http.Error(w, "bad upstream request", http.StatusInternalServerError) + return + } + // frame.jpg takes width and quality; the engine re-encodes on demand. + req.URL.RawQuery = r.URL.RawQuery + if user != "" { + req.SetBasicAuth(user, pass) + } + + resp, err := client.Do(req) + if err != nil { + http.Error(w, "engine unreachable", http.StatusBadGateway) + return + } + defer resp.Body.Close() + + for _, h := range []string{"Content-Type", "Cache-Control", "Pragma", "Expires"} { + if v := resp.Header.Get(h); v != "" { + w.Header().Set(h, v) + } + } + w.WriteHeader(resp.StatusCode) + + // Copied by hand rather than with io.Copy so every chunk is flushed. An + // MJPEG stream never ends, so anything buffered waiting for a full buffer + // is a tile that stays blank forever - which is the same symptom as the + // bug this file exists to fix, and would look like it had not worked. + flusher, _ := w.(http.Flusher) + buf := make([]byte, 32*1024) + for { + n, rerr := resp.Body.Read(buf) + if n > 0 { + if _, werr := w.Write(buf[:n]); werr != nil { + return // webview went away + } + if flusher != nil { + flusher.Flush() + } + } + if rerr != nil { + return + } + } +} diff --git a/desktop/stream_proxy_test.go b/desktop/stream_proxy_test.go new file mode 100644 index 0000000..b44877e --- /dev/null +++ b/desktop/stream_proxy_test.go @@ -0,0 +1,296 @@ +package main + +import ( + "fmt" + "io" + "net/http" + "net/http/httptest" + "os" + "strings" + "testing" + "time" +) + +// fakeEngine stands in for the Python engine: it demands Basic auth exactly as +// the real one does when a credential is configured, and records what it was +// asked for. +type fakeEngine struct { + *httptest.Server + gotPath string + gotUser string + gotPass string + hadAuth bool +} + +func newFakeEngine(t *testing.T, body string) *fakeEngine { + t.Helper() + f := &fakeEngine{} + f.Server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + f.gotPath = r.URL.Path + if r.URL.RawQuery != "" { + f.gotPath += "?" + r.URL.RawQuery + } + f.gotUser, f.gotPass, f.hadAuth = r.BasicAuth() + if !f.hadAuth { + w.Header().Set("WWW-Authenticate", `Basic realm="behavision"`) + w.WriteHeader(http.StatusUnauthorized) + return + } + w.Header().Set("Content-Type", "multipart/x-mixed-replace; boundary=frame") + _, _ = io.WriteString(w, body) + })) + t.Cleanup(f.Close) + return f +} + +func startProxy(t *testing.T, engine string, user, pass string) *streamProxy { + t.Helper() + p := newStreamProxy() + if err := p.start(engine, user, pass); err != nil { + t.Fatalf("start: %v", err) + } + t.Cleanup(p.stop) + return p +} + +func get(t *testing.T, url string) (int, string) { + t.Helper() + c := &http.Client{Timeout: 5 * time.Second} + resp, err := c.Get(url) + if err != nil { + t.Fatalf("get %s: %v", url, err) + } + defer resp.Body.Close() + b, _ := io.ReadAll(resp.Body) + return resp.StatusCode, string(b) +} + +// The whole point: the webview gets a URL it can actually load, and the +// password stays behind. A credential in the src is both unloadable in a +// Chromium webview and readable by anything that can see the DOM. +func TestTheCameraURLCarriesNoPassword(t *testing.T) { + engine := newFakeEngine(t, "frames") + p := startProxy(t, engine.URL, "behavision", "hunter2-the-real-one") + + u := p.urlFor("cam2", "stream.mjpeg") + if u == "" { + t.Fatal("no url while the proxy is running") + } + if strings.Contains(u, "hunter2-the-real-one") || strings.Contains(u, "behavision:") { + t.Fatalf("credential leaked into the tile URL: %s", u) + } + if !strings.HasPrefix(u, "http://127.0.0.1:") { + t.Fatalf("relay must be loopback only, got %s", u) + } +} + +func TestTheRelayAttachesTheCredentialItself(t *testing.T) { + engine := newFakeEngine(t, "frame-bytes") + p := startProxy(t, engine.URL, "behavision", "s3cret") + + code, body := get(t, p.urlFor("cam2", "stream.mjpeg")) + if code != http.StatusOK { + t.Fatalf("want 200 through the relay, got %d", code) + } + if body != "frame-bytes" { + t.Fatalf("body not relayed unchanged: %q", body) + } + if !engine.hadAuth || engine.gotUser != "behavision" || engine.gotPass != "s3cret" { + t.Fatalf("engine did not receive the credential: auth=%v user=%q", + engine.hadAuth, engine.gotUser) + } + if engine.gotPath != "/api/cameras/cam2/stream.mjpeg" { + t.Fatalf("wrong upstream path: %s", engine.gotPath) + } +} + +// The token is what keeps every other process on a shop PC from opening a live +// view of customers' faces, now that the relay itself has no password. +func TestAnotherProcessCannotGuessItsWayIn(t *testing.T) { + engine := newFakeEngine(t, "frames") + p := startProxy(t, engine.URL, "behavision", "s3cret") + addr := p.ln.Addr().String() + + for _, bad := range []string{"", "0", strings.Repeat("a", 64), "wrong-token"} { + url := fmt.Sprintf("http://%s/s/%s/cam2/stream.mjpeg", addr, bad) + if code, _ := get(t, url); code != http.StatusNotFound { + t.Fatalf("token %q got %d, want 404", bad, code) + } + } + if engine.hadAuth { + t.Fatal("a rejected request still reached the engine") + } +} + +// A camera id is interpolated into the upstream path, so it has to be a camera +// id and not a way to walk to a different endpoint with the credential +// attached. +func TestACameraIdCannotClimbOutOfItsPath(t *testing.T) { + engine := newFakeEngine(t, "frames") + p := startProxy(t, engine.URL, "behavision", "s3cret") + addr := p.ln.Addr().String() + + for _, bad := range []string{"..", "%2e%2e", "cam2/../../api/identities", "cam 2", ""} { + url := fmt.Sprintf("http://%s/s/%s/%s/stream.mjpeg", addr, p.token, bad) + code, _ := get(t, url) + if code != http.StatusNotFound { + t.Fatalf("camera id %q got %d, want 404", bad, code) + } + } + if strings.Contains(engine.gotPath, "identities") { + t.Fatalf("reached a non-camera endpoint: %s", engine.gotPath) + } +} + +// Only the two files a tile needs. The engine also serves the identity list and +// the erasure endpoint; holding the token must not open those. +func TestOnlyTheTwoCameraFilesAreReachable(t *testing.T) { + engine := newFakeEngine(t, "frames") + p := startProxy(t, engine.URL, "behavision", "s3cret") + addr := p.ln.Addr().String() + + for _, bad := range []string{"identities", "stats", "commission", "stream.mjpeg.bak"} { + url := fmt.Sprintf("http://%s/s/%s/cam2/%s", addr, p.token, bad) + if code, _ := get(t, url); code != http.StatusNotFound { + t.Fatalf("file %q got %d, want 404", bad, code) + } + } + + for _, good := range []string{"stream.mjpeg", "frame.jpg"} { + url := fmt.Sprintf("http://%s/s/%s/cam2/%s", addr, p.token, good) + if code, _ := get(t, url); code != http.StatusOK { + t.Fatalf("file %q got %d, want 200", good, code) + } + } +} + +// frame.jpg takes width and quality - the engine re-encodes on demand, and a +// relay that dropped the query would silently serve full-size frames. +func TestTheQueryStringSurvivesTheRelay(t *testing.T) { + engine := newFakeEngine(t, "frames") + p := startProxy(t, engine.URL, "behavision", "s3cret") + + url := p.urlFor("cam2", "frame.jpg") + "?width=640&quality=70" + if code, _ := get(t, url); code != http.StatusOK { + t.Fatalf("got %d", code) + } + if !strings.Contains(engine.gotPath, "width=640") || + !strings.Contains(engine.gotPath, "quality=70") { + t.Fatalf("query dropped: %s", engine.gotPath) + } +} + +// An engine that is not running must read as a bad gateway, not as a hang. A +// blank tile that never resolves is the symptom this whole file exists to end. +func TestAnEngineThatIsDownFailsQuickly(t *testing.T) { + // Port 1 on loopback: nothing listens, and the connection is refused + // rather than dropped, so this is fast and deterministic. + p := startProxy(t, "http://127.0.0.1:1", "behavision", "s3cret") + + done := make(chan int, 1) + go func() { code, _ := get(t, p.urlFor("cam2", "stream.mjpeg")); done <- code }() + select { + case code := <-done: + if code != http.StatusBadGateway { + t.Fatalf("want 502, got %d", code) + } + case <-time.After(8 * time.Second): + t.Fatal("a dead engine left the request hanging") + } +} + +// Stopping must actually free the port, or a restarted engine leaves listeners +// behind for the life of the process. +func TestStoppingReleasesEverything(t *testing.T) { + engine := newFakeEngine(t, "frames") + p := newStreamProxy() + if err := p.start(engine.URL, "u", "p"); err != nil { + t.Fatalf("start: %v", err) + } + url := p.urlFor("cam2", "stream.mjpeg") + if code, _ := get(t, url); code != http.StatusOK { + t.Fatalf("want 200 before stop, got %d", code) + } + + p.stop() + + if got := p.urlFor("cam2", "stream.mjpeg"); got != "" { + t.Fatalf("still handing out URLs after stop: %s", got) + } + c := &http.Client{Timeout: 3 * time.Second} + if resp, err := c.Get(url); err == nil { + resp.Body.Close() + t.Fatal("listener still accepting after stop") + } +} + +// start twice must not leave two listeners, which is what a restarted engine +// would otherwise cause. +func TestStartingTwiceIsANoOp(t *testing.T) { + engine := newFakeEngine(t, "frames") + p := startProxy(t, engine.URL, "u", "p") + + first := p.urlFor("cam2", "stream.mjpeg") + if err := p.start(engine.URL, "u", "p"); err != nil { + t.Fatalf("second start: %v", err) + } + if second := p.urlFor("cam2", "stream.mjpeg"); second != first { + t.Fatalf("second start moved the relay: %s -> %s", first, second) + } +} + +// Against the real engine, which the unit tests above deliberately do not +// touch. Skipped unless TEST_ENGINE_URL is set, the same rule the server's +// live store tests follow: the suite must stay runnable with no services. +// +// TEST_ENGINE_URL=http://127.0.0.1:8010 \ +// TEST_ENGINE_USER=... TEST_ENGINE_PASS=... go test ./desktop/ -run Live +// +// It exists because everything above proves the relay against a fake that +// agrees with me. Only a real engine proves the thing that was actually +// broken: that a multipart MJPEG stream arrives through the relay in pieces, +// rather than being buffered into a tile that never paints. +func TestLiveRelayCarriesRealMJPEGFrames(t *testing.T) { + base := os.Getenv("TEST_ENGINE_URL") + if base == "" { + t.Skip("set TEST_ENGINE_URL to run the live relay test") + } + cam := os.Getenv("TEST_ENGINE_CAMERA") + if cam == "" { + cam = "cam2" + } + p := startProxy(t, base, os.Getenv("TEST_ENGINE_USER"), os.Getenv("TEST_ENGINE_PASS")) + + url := p.urlFor(cam, "stream.mjpeg") + req, _ := http.NewRequest(http.MethodGet, url, nil) + resp, err := (&http.Client{}).Do(req) + if err != nil { + t.Fatalf("relay: %v", err) + } + defer resp.Body.Close() + if resp.StatusCode != http.StatusOK { + t.Fatalf("relay returned %d - the credential did not reach the engine", resp.StatusCode) + } + if ct := resp.Header.Get("Content-Type"); !strings.Contains(ct, "multipart") { + t.Fatalf("not a stream: Content-Type %q", ct) + } + + // Read until two JPEG start markers have gone past. One proves it opened; + // two prove it is still delivering, which is the difference between a + // working tile and a single frozen frame. + deadline := time.Now().Add(15 * time.Second) + var seen, total int + buf := make([]byte, 16*1024) + for seen < 2 && time.Now().Before(deadline) { + n, rerr := resp.Body.Read(buf) + total += n + seen += strings.Count(string(buf[:n]), "\xff\xd8\xff") + if rerr != nil { + break + } + } + if seen < 2 { + t.Fatalf("only %d JPEG frames in %d bytes - the relay is not streaming", seen, total) + } + t.Logf("relayed %d frames in %d bytes with no credential in the URL", seen, total) +}