From 4db93000013d1c483722ceb6baaedabc26e39fca Mon Sep 17 00:00:00 2001 From: Juan Denis Date: Tue, 5 May 2026 14:03:31 -0400 Subject: [PATCH] Introduce sk1 connection string format and GUI plugin Switch connection string encoding from the old base64 blob to a readable sk1:// URL format (host/token[?exp&insecure]) across agent, backend, and frontend. Update encode/decode logic and extensive tests to validate port preservation, insecure/http handling, expiry parsing, and input validation; adjust agent pairing UI to accept the new scheme. Add ServerKit Agent GUI plugin: backend blueprint endpoints for capabilities, frame capture and synthetic desktop, plugin metadata; frontend plugin components (ServerGui, Launcher, SyntheticDesktop), styles, and manifest entry to enable streaming screenshots with a synthetic fallback. Also add Windows service stop waiting to avoid start races after pairing, and document SERVERKIT_PUBLIC_URL in env examples. --- .env.example | 7 + agent/internal/agentui/pairer.go | 11 +- agent/internal/agentui/platform_windows.go | 22 + agent/internal/connstring/connstring.go | 110 ++--- agent/internal/connstring/connstring_test.go | 104 +++-- agent/ui/src/pages/Pair.jsx | 4 +- backend/.env.example | 6 + backend/app/plugins/serverkit-gui/__init__.py | 3 + .../app/plugins/serverkit-gui/blueprint.py | 199 +++++++++ backend/app/plugins/serverkit-gui/plugin.json | 31 ++ backend/app/services/connection_string.py | 135 ++++--- backend/tests/test_connection_string.py | 108 ++++- backend/tests/test_servers_api_pairing.py | 4 +- frontend/src/plugins/plugins-manifest.json | 13 +- .../serverkit-gui/components/ServerGui.jsx | 273 +++++++++++++ .../components/ServerGuiLauncher.jsx | 68 ++++ .../components/SyntheticDesktop.jsx | 91 +++++ frontend/src/plugins/serverkit-gui/index.js | 16 + .../src/plugins/serverkit-gui/plugin.json | 31 ++ .../serverkit-gui/styles/server-gui.css | 377 ++++++++++++++++++ frontend/vite.config.js | 9 + 21 files changed, 1444 insertions(+), 178 deletions(-) create mode 100644 backend/app/plugins/serverkit-gui/__init__.py create mode 100644 backend/app/plugins/serverkit-gui/blueprint.py create mode 100644 backend/app/plugins/serverkit-gui/plugin.json create mode 100644 frontend/src/plugins/serverkit-gui/components/ServerGui.jsx create mode 100644 frontend/src/plugins/serverkit-gui/components/ServerGuiLauncher.jsx create mode 100644 frontend/src/plugins/serverkit-gui/components/SyntheticDesktop.jsx create mode 100644 frontend/src/plugins/serverkit-gui/index.js create mode 100644 frontend/src/plugins/serverkit-gui/plugin.json create mode 100644 frontend/src/plugins/serverkit-gui/styles/server-gui.css diff --git a/.env.example b/.env.example index 70b8bbdd..84ed8843 100644 --- a/.env.example +++ b/.env.example @@ -30,6 +30,13 @@ DATABASE_URL=sqlite:///serverkit.db # CORS Origins - your domain(s), comma-separated CORS_ORIGINS=http://localhost,https://your-domain.com +# Public URL that agents should dial back to. Set this whenever the panel +# sits behind a reverse proxy / tunnel / custom domain — the connection +# string the panel issues to agents will embed this URL instead of the +# request's Host header (which would otherwise be the internal IP/port). +# Leave commented out for plain localhost development. +# SERVERKIT_PUBLIC_URL=https://panel.your-domain.com + # Ports (for Docker) PORT=80 SSL_PORT=443 diff --git a/agent/internal/agentui/pairer.go b/agent/internal/agentui/pairer.go index aec8af88..c50f18dd 100644 --- a/agent/internal/agentui/pairer.go +++ b/agent/internal/agentui/pairer.go @@ -131,6 +131,13 @@ func (p *pairer) handleStart(w http.ResponseWriter, r *http.Request) { if err := runServiceCmd("stop"); err != nil { p.log.Info("Service stop reported error (likely already stopped)", "error", err) } + // sc.exe returns once SCM accepts the stop request, not when the + // service is fully stopped. Issuing sc start while the service is + // still STOP_PENDING fails with 1056 ("instance already running"), + // so wait until SCM reports STOPPED before starting. + if err := waitForServiceStopped(15 * time.Second); err != nil { + p.log.Info("Service did not reach STOPPED before start", "error", err) + } if err := runServiceCmd("start"); err != nil { p.log.Error("Failed to start service after pairing", "error", err) p.mu.Lock() @@ -213,8 +220,8 @@ type connStringRequest struct { ConnectionString string `json:"connection_string"` } -// handleConnectionString accepts a single sk_conn_v1.<...> blob, decodes -// it into a panel URL + registration token, and runs the legacy +// handleConnectionString accepts a single sk1://… connection string, +// decodes it into a panel URL + registration token, and runs the legacy // /api/v1/servers/register flow against the panel — the same one the // `serverkit-agent register` CLI uses. We reuse the existing pairer // state machine ("enrolling" -> "claimed") so the React wizard's polling diff --git a/agent/internal/agentui/platform_windows.go b/agent/internal/agentui/platform_windows.go index feed0ad7..fa7c1862 100644 --- a/agent/internal/agentui/platform_windows.go +++ b/agent/internal/agentui/platform_windows.go @@ -49,6 +49,28 @@ func waitForServiceRunning(timeout time.Duration) error { timeout, strings.TrimSpace(lastOut)) } +// waitForServiceStopped polls `sc query` until the service reports state +// 1 (STOPPED) or the deadline passes. `sc stop` returns as soon as SCM +// accepts the stop request — issuing `sc start` while the service is +// still STOP_PENDING returns error 1056 ("instance already running"), +// which is exactly what users hit on the post-pair restart. +func waitForServiceStopped(timeout time.Duration) error { + deadline := time.Now().Add(timeout) + var lastOut string + for time.Now().Before(deadline) { + cmd := exec.Command("sc.exe", "query", "ServerKitAgent") + cmd.SysProcAttr = &syscall.SysProcAttr{HideWindow: true} + out, err := cmd.CombinedOutput() + lastOut = string(out) + if err == nil && strings.Contains(lastOut, "STOPPED") { + return nil + } + time.Sleep(500 * time.Millisecond) + } + return fmt.Errorf("service did not reach STOPPED within %s (last sc query: %s)", + timeout, strings.TrimSpace(lastOut)) +} + // openTarget hands a path or URL to Explorer / the default browser via // rundll32 + url.dll, which is the canonical "open this thing" entry point // on Windows. Faster than shelling out to cmd /c start. diff --git a/agent/internal/connstring/connstring.go b/agent/internal/connstring/connstring.go index 80e98003..91af9e95 100644 --- a/agent/internal/connstring/connstring.go +++ b/agent/internal/connstring/connstring.go @@ -1,92 +1,102 @@ // Package connstring decodes the panel's "connection string" — a single -// base64-packed blob the user copies from the ServerKit panel and pastes -// into the agent's pairing wizard. It bundles the panel URL plus a -// single-use registration token, replacing the older flow where the user -// typed those into separate fields. +// URL-shaped string the user copies from the ServerKit panel and pastes +// into the agent's pairing wizard. It bundles the panel host plus a +// single-use registration token plus an optional expiry, replacing the +// older flow where the user typed those into separate fields. // -// Format: ``sk_conn_v1.`` +// Format: ``sk1://[:]/[?exp=][&insecure=1]`` // -// The version prefix exists so older agents reject newer payloads cleanly -// instead of mis-parsing them. Today's payload is intentionally tiny — -// just the URL, the token, and an optional expiry the agent treats as -// advisory (the panel is the authoritative source of truth on whether the -// token is still good). +// The ``sk1://`` scheme self-identifies the format (you can recognise a +// ServerKit connection string at a glance) and gives us a version +// lever: any future format change goes out as ``sk2://``. The host is +// visible up front so the user can sanity-check which panel they're +// pointing at before pasting. ``insecure=1`` flips the implied scheme +// from https to http for dev / local-network use; absent it, https is +// implied. Older agents reject newer-versioned payloads cleanly with +// ErrUnknownVersion instead of mis-parsing them. package connstring import ( - "encoding/base64" - "encoding/json" "errors" "fmt" + "net/url" "strings" "time" ) const ( - // Prefix is the version tag that introduces every v1 connection string. - Prefix = "sk_conn_v1." + // SchemePrefix is the scheme + separator that introduces every v1 + // connection string. Used to fail fast on legacy or unrelated input + // before we try to parse it as a URL. + SchemePrefix = "sk1://" ) -// Decoded is the payload encoded inside a connection string. +// Decoded is the data extracted from a connection string. type Decoded struct { - URL string `json:"url"` - Token string `json:"token"` - ExpiresAt time.Time `json:"-"` // parsed from the raw payload below - // Raw expiry kept verbatim so we can show the user the panel's exact - // formatting (or "never") without re-stringifying. - ExpiresAtRaw string `json:"expires_at,omitempty"` + URL string // reconstructed http/https URL of the panel + Token string // single-use registration token + ExpiresAt time.Time // zero value means "never" or "unparseable" + ExpiresAtRaw string // panel's exact expiry string (or "" if none) } // ErrUnknownVersion is returned when the prefix is missing or names a // version this build doesn't understand. Callers should surface this as -// "your panel is newer than this agent" rather than a generic decode error -// — it's almost always the cause. +// "your panel is newer than this agent" rather than a generic decode +// error — it's almost always the cause. var ErrUnknownVersion = errors.New("connstring: unknown version prefix") -// Decode parses a connection string. Whitespace surrounding the input is -// stripped (paste-from-clipboard often picks up a trailing newline). +// Decode parses a connection string. Whitespace surrounding the input +// is stripped (paste-from-clipboard often picks up a trailing newline). func Decode(s string) (*Decoded, error) { s = strings.TrimSpace(s) if s == "" { return nil, errors.New("connstring: empty input") } - if !strings.HasPrefix(s, Prefix) { + if !strings.HasPrefix(s, SchemePrefix) { return nil, ErrUnknownVersion } - payload := strings.TrimPrefix(s, Prefix) - // We use the URL-safe alphabet without padding on the panel side; allow - // either padded or unpadded inputs here so a hand-edited string still - // decodes. - raw, err := base64.RawURLEncoding.DecodeString(payload) + u, err := url.Parse(s) if err != nil { - raw, err = base64.URLEncoding.DecodeString(payload) + return nil, fmt.Errorf("connstring: parse: %w", err) } - if err != nil { - return nil, fmt.Errorf("connstring: base64 decode: %w", err) + if u.Scheme != "sk1" { + return nil, ErrUnknownVersion + } + if u.Host == "" { + return nil, errors.New("connstring: missing host") } - // Parse twice — once into the public Decoded struct (gets URL+Token), - // then into a small auxiliary struct to capture the expiry as both a - // time.Time and the original string. - var out Decoded - if err := json.Unmarshal(raw, &out); err != nil { - return nil, fmt.Errorf("connstring: json decode: %w", err) + token := strings.TrimPrefix(u.Path, "/") + if token == "" { + return nil, errors.New("connstring: missing token") } - if out.URL == "" || out.Token == "" { - return nil, errors.New("connstring: payload missing url or token") + // Tokens are url-safe by construction (panel uses secrets.token_urlsafe); + // a stray slash means the user mangled the string. Better to fail + // loudly than silently truncate. + if strings.Contains(token, "/") { + return nil, errors.New("connstring: token must not contain '/'") } - var aux struct { - ExpiresAt string `json:"expires_at"` + q := u.Query() + scheme := "https" + switch q.Get("insecure") { + case "1", "true", "yes": + scheme = "http" } - _ = json.Unmarshal(raw, &aux) - out.ExpiresAtRaw = aux.ExpiresAt - if aux.ExpiresAt != "" { - if t, perr := time.Parse(time.RFC3339, aux.ExpiresAt); perr == nil { + + out := &Decoded{ + URL: scheme + "://" + u.Host, + Token: token, + } + if exp := q.Get("exp"); exp != "" { + out.ExpiresAtRaw = exp + // Best-effort parse — if the panel sends a format we don't + // understand we still surface the raw value, and the panel + // remains the source of truth on whether the token is good. + if t, perr := time.Parse(time.RFC3339, exp); perr == nil { out.ExpiresAt = t } } - - return &out, nil + return out, nil } diff --git a/agent/internal/connstring/connstring_test.go b/agent/internal/connstring/connstring_test.go index 37612e7f..e76f8337 100644 --- a/agent/internal/connstring/connstring_test.go +++ b/agent/internal/connstring/connstring_test.go @@ -1,22 +1,13 @@ package connstring import ( - "encoding/base64" "errors" "strings" "testing" ) -// helper: encode the panel's payload format so the test stays readable -// even as we tweak fields. Mirrors backend/app/services/connection_string.py -// — keep the two in sync. -func encode(payload string) string { - return Prefix + base64.RawURLEncoding.EncodeToString([]byte(payload)) -} - func TestDecode_Roundtrip(t *testing.T) { - s := encode(`{"url":"https://panel.example.com","token":"sk_reg_abc","expires_at":"2026-05-08T17:00:00Z"}`) - got, err := Decode(s) + got, err := Decode("sk1://panel.example.com/sk_reg_abc?exp=2026-05-08T17:00:00Z") if err != nil { t.Fatalf("decode: %v", err) } @@ -34,24 +25,61 @@ func TestDecode_Roundtrip(t *testing.T) { } } +func TestDecode_PreservesPort(t *testing.T) { + got, err := Decode("sk1://panel.example.com:9443/t") + if err != nil { + t.Fatalf("decode: %v", err) + } + if got.URL != "https://panel.example.com:9443" { + t.Errorf("url = %q", got.URL) + } +} + +func TestDecode_HonoursInsecureFlag(t *testing.T) { + // http panels (typically dev / local-network) round-trip via the + // insecure=1 flag. Without honouring it, the agent would default + // to https and fail TLS against an http-only backend. + got, err := Decode("sk1://localhost:47927/t?insecure=1") + if err != nil { + t.Fatalf("decode: %v", err) + } + if got.URL != "http://localhost:47927" { + t.Errorf("url = %q (want http scheme)", got.URL) + } +} + +func TestDecode_NoExpiry(t *testing.T) { + got, err := Decode("sk1://panel.example.com/t") + if err != nil { + t.Fatalf("decode: %v", err) + } + if got.ExpiresAtRaw != "" { + t.Errorf("expected no raw expiry, got %q", got.ExpiresAtRaw) + } + if !got.ExpiresAt.IsZero() { + t.Errorf("expected zero ExpiresAt, got %v", got.ExpiresAt) + } +} + func TestDecode_TrimsWhitespace(t *testing.T) { - s := encode(`{"url":"https://x","token":"t"}`) // Clipboard pastes often have a trailing newline; verify Decode // doesn't reject a string just because of that. - if _, err := Decode("\n " + s + " \n"); err != nil { + if _, err := Decode("\n sk1://panel.example.com/t \n"); err != nil { t.Fatalf("decode with whitespace: %v", err) } } func TestDecode_RejectsUnknownVersion(t *testing.T) { - _, err := Decode("sk_conn_v9.aaaa") - if !errors.Is(err, ErrUnknownVersion) { - t.Errorf("want ErrUnknownVersion, got %v", err) + cases := []string{ + "sk2://panel.example.com/t", + "sk_conn_v1.eyJ1cmwiOiJodHRwczovL3gifQ", // legacy format + "not-a-connection-string", } - - _, err = Decode("not-a-connection-string") - if !errors.Is(err, ErrUnknownVersion) { - t.Errorf("want ErrUnknownVersion for missing prefix, got %v", err) + for _, in := range cases { + _, err := Decode(in) + if !errors.Is(err, ErrUnknownVersion) { + t.Errorf("Decode(%q): want ErrUnknownVersion, got %v", in, err) + } } } @@ -64,39 +92,33 @@ func TestDecode_RejectsEmpty(t *testing.T) { } } -func TestDecode_RejectsMissingFields(t *testing.T) { +func TestDecode_RejectsMissingToken(t *testing.T) { cases := []string{ - `{"token":"t"}`, - `{"url":"https://x"}`, - `{}`, + "sk1://panel.example.com", + "sk1://panel.example.com/", } - for _, payload := range cases { - _, err := Decode(encode(payload)) + for _, in := range cases { + _, err := Decode(in) if err == nil { - t.Errorf("expected error for payload %q", payload) + t.Errorf("Decode(%q): want error, got nil", in) continue } - if !strings.Contains(err.Error(), "missing url or token") { - t.Errorf("payload %q: unexpected error %v", payload, err) + if !strings.Contains(err.Error(), "token") { + t.Errorf("Decode(%q): unexpected error %v", in, err) } } } -func TestDecode_RejectsCorruptBase64(t *testing.T) { - // Garbage after the prefix isn't valid base64 in either alphabet. - _, err := Decode(Prefix + "!!!not-base64!!!") - if err == nil { - t.Errorf("want error for corrupt base64") +func TestDecode_RejectsTokenWithSlash(t *testing.T) { + _, err := Decode("sk1://panel.example.com/sk_reg/abc") + if err == nil || !strings.Contains(err.Error(), "/") { + t.Errorf("expected error about '/', got %v", err) } } -func TestDecode_AcceptsPaddedBase64(t *testing.T) { - // Some clipboards (notably pasting through certain terminals) round- - // trip the URL-safe alphabet through a padded form. Decode tolerates - // either, so a hand-edited string still works. - payload := []byte(`{"url":"https://x","token":"t"}`) - padded := Prefix + base64.URLEncoding.EncodeToString(payload) - if _, err := Decode(padded); err != nil { - t.Errorf("padded decode: %v", err) +func TestDecode_RejectsMissingHost(t *testing.T) { + _, err := Decode("sk1:///t") + if err == nil || !strings.Contains(err.Error(), "host") { + t.Errorf("expected host error, got %v", err) } } diff --git a/agent/ui/src/pages/Pair.jsx b/agent/ui/src/pages/Pair.jsx index cda41380..34d49eec 100644 --- a/agent/ui/src/pages/Pair.jsx +++ b/agent/ui/src/pages/Pair.jsx @@ -47,7 +47,7 @@ function FormStage({ onConnectionString, onPairCode, busy, defaultName }) { setError('Paste the connection string from your panel.'); return; } - if (!trimmed.startsWith('sk_conn_')) { + if (!trimmed.startsWith('sk1://')) { setError('That doesn’t look like a connection string. Generate one from the panel’s Add Server screen.'); return; } @@ -103,7 +103,7 @@ function FormStage({ onConnectionString, onPairCode, busy, defaultName }) {