diff --git a/apps/backend/internal/sentry/models.go b/apps/backend/internal/sentry/models.go index 631e17f744..6ceebfa435 100644 --- a/apps/backend/internal/sentry/models.go +++ b/apps/backend/internal/sentry/models.go @@ -20,7 +20,11 @@ const SecretKey = "sentry:singleton:token" // The token is stored separately in the encrypted secret store under SecretKey. type SentryConfig struct { AuthMethod string `json:"authMethod" db:"auth_method"` - HasSecret bool `json:"hasSecret" db:"-"` + // URL is the base URL of the Sentry instance (e.g. https://sentry.io for + // SaaS, or a self-hosted host). The REST client appends /api/0. Defaults + // to the sentry.io SaaS endpoint when left blank. + URL string `json:"url" db:"url"` + HasSecret bool `json:"hasSecret" db:"-"` // LastCheckedAt / LastOk / LastError are written by the background auth // poller so the UI can render a "connected/disconnected + checked Xs ago" // indicator without doing its own probing. @@ -36,7 +40,10 @@ type SentryConfig struct { // is retained; when non-empty it replaces the stored value. type SetConfigRequest struct { AuthMethod string `json:"authMethod"` - Secret string `json:"secret"` + // URL is the Sentry instance base URL. Optional: blank defaults to the + // sentry.io SaaS endpoint, preserving the prior single-tenant behavior. + URL string `json:"url"` + Secret string `json:"secret"` } // TestConnectionResult reports what the backend learned when pinging Sentry diff --git a/apps/backend/internal/sentry/rest_client.go b/apps/backend/internal/sentry/rest_client.go index a5216d84e0..86e2cf352f 100644 --- a/apps/backend/internal/sentry/rest_client.go +++ b/apps/backend/internal/sentry/rest_client.go @@ -13,31 +13,56 @@ import ( "time" ) -// SentryAPIEndpoint is the SaaS API base. Phase 1 targets sentry.io only; -// self-hosted instances will need a per-config base-URL field later. -const SentryAPIEndpoint = "https://sentry.io/api/0" +// DefaultSentryURL is the SaaS base URL used when no custom instance URL is +// configured. Self-hosted installs override it with their own host; the REST +// client appends apiPathPrefix to whichever base it is given. +const DefaultSentryURL = "https://sentry.io" + +// apiPathPrefix is appended to the instance base URL to reach the REST API. +const apiPathPrefix = "/api/0" const userAgent = "kandev/1.0 (+https://github.com/kdlbs/kandev)" // RESTClient talks to Sentry's REST API using a Bearer auth token. type RESTClient struct { http *http.Client - endpoint string + endpoint string // REST API base, including the apiPathPrefix suffix + baseURL string // instance base URL shown to users (no apiPathPrefix) token string maxBodySize int64 } -// NewRESTClient builds a client from a SentryConfig + secret. The config is -// accepted for symmetry with other integrations; only the token is read today. -func NewRESTClient(_ *SentryConfig, secret string) *RESTClient { +// NewRESTClient builds a client from a SentryConfig + secret. The config's URL +// selects the instance (SaaS or self-hosted); a blank URL falls back to +// DefaultSentryURL so existing single-tenant installs keep working. +func NewRESTClient(cfg *SentryConfig, secret string) *RESTClient { + rawURL := "" + if cfg != nil { + rawURL = cfg.URL + } + base := resolveBaseURL(rawURL) return &RESTClient{ http: &http.Client{Timeout: 30 * time.Second}, - endpoint: SentryAPIEndpoint, + endpoint: base + apiPathPrefix, + baseURL: base, token: secret, maxBodySize: 8 << 20, // 8 MB — issue payloads can include event samples. } } +// resolveBaseURL normalizes an instance base URL (scheme defaulted, trailing +// slash trimmed), falling back to the SaaS default when empty. The result is +// the user-facing host root; apiPathPrefix is appended separately to form the +// REST endpoint, so it is kept out of error messages to avoid users pasting a +// /api/0-suffixed value back into the Instance URL field. +func resolveBaseURL(rawURL string) string { + base := normalizeSentryURL(rawURL) + if base == "" { + base = DefaultSentryURL + } + return base +} + // do executes a GET against path (relative to endpoint), parses query params, // and decodes the JSON body into out. Returns the response so callers can read // headers (e.g. Link for pagination). @@ -104,11 +129,7 @@ type whoamiResponse struct { func (c *RESTClient) TestAuth(ctx context.Context) (*TestConnectionResult, error) { var data whoamiResponse if _, err := c.do(ctx, "/", nil, &data); err != nil { - var apiErr *APIError - if errors.As(err, &apiErr) { - return &TestConnectionResult{OK: false, Error: apiErr.Error()}, nil - } - return &TestConnectionResult{OK: false, Error: err.Error()}, nil + return &TestConnectionResult{OK: false, Error: c.classifyProbeError(err)}, nil } name := data.User.Name if name == "" { @@ -122,6 +143,25 @@ func (c *RESTClient) TestAuth(ctx context.Context) (*TestConnectionResult, error }, nil } +// classifyProbeError turns a /api/0/ probe failure into a user-facing message +// that separates a credential problem from an instance-URL problem. With +// self-hosted support the URL is a common misconfiguration, so a transport +// failure (host unreachable / wrong scheme) or a non-Sentry HTTP response +// (any status other than 401/403, including a 200 whose body isn't the +// expected JSON) blames the configured URL, while 401/403 blames the token. +func (c *RESTClient) classifyProbeError(err error) string { + var apiErr *APIError + if errors.As(err, &apiErr) { + switch apiErr.StatusCode { + case http.StatusUnauthorized, http.StatusForbidden: + return fmt.Sprintf("authentication failed — check the auth token (HTTP %d)", apiErr.StatusCode) + default: + return fmt.Sprintf("%s did not respond like a Sentry API (HTTP %d) — check the instance URL", c.baseURL, apiErr.StatusCode) + } + } + return fmt.Sprintf("could not reach Sentry at %s — check the instance URL (%s)", c.baseURL, err.Error()) +} + // --- organizations --- // listOrganizationsMaxPages bounds the pagination loop so a misbehaving API or diff --git a/apps/backend/internal/sentry/rest_client_test.go b/apps/backend/internal/sentry/rest_client_test.go index 27997be225..757a800ca8 100644 --- a/apps/backend/internal/sentry/rest_client_test.go +++ b/apps/backend/internal/sentry/rest_client_test.go @@ -19,6 +19,7 @@ func newMockServer(t *testing.T, handler http.HandlerFunc) *httptest.Server { // httptest server without needing a mockable URL on the production constructor. func pointTo(c *RESTClient, url string) *RESTClient { c.endpoint = url + c.baseURL = url return c } @@ -275,3 +276,96 @@ func TestBuildIssueQueryString(t *testing.T) { } } } + +// TestNewRESTClient_BuildsEndpointFromConfigURL locks in that a self-hosted +// instance URL becomes the API base with the /api/0 suffix appended, that a +// trailing slash and missing scheme are normalized, and that an empty URL +// falls back to the SaaS default. +func TestNewRESTClient_BuildsEndpointFromConfigURL(t *testing.T) { + cases := []struct { + name string + url string + want string + }{ + {"self-hosted", "https://sentry.example.com", "https://sentry.example.com/api/0"}, + {"trailing slash", "https://sentry.example.com/", "https://sentry.example.com/api/0"}, + {"scheme defaulted", "sentry.example.com", "https://sentry.example.com/api/0"}, + {"empty falls back to saas", "", DefaultSentryURL + "/api/0"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + c := NewRESTClient(&SentryConfig{URL: tc.url}, "tok") + if c.endpoint != tc.want { + t.Errorf("endpoint = %q, want %q", c.endpoint, tc.want) + } + }) + } +} + +// TestRESTClient_TestAuth_WrongURL covers the self-hosted failure mode: when +// the configured instance answers but isn't a Sentry API (non-JSON body, or a +// 5xx/4xx other than 401/403), the probe must blame the instance URL rather +// than the auth token. +func TestRESTClient_TestAuth_WrongURL(t *testing.T) { + t.Run("non-json body", func(t *testing.T) { + ts := newMockServer(t, func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "text/html") + _, _ = w.Write([]byte("Not Sentry")) + }) + c := pointTo(NewRESTClient(&SentryConfig{}, "tok"), ts.URL) + res, err := c.TestAuth(context.Background()) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + if res.OK { + t.Error("expected OK=false for non-Sentry response") + } + if !strings.Contains(res.Error, "instance URL") { + t.Errorf("expected instance-URL hint, got %q", res.Error) + } + }) + t.Run("server error", func(t *testing.T) { + ts := newMockServer(t, func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusBadGateway) + _, _ = w.Write([]byte("bad gateway")) + }) + c := pointTo(NewRESTClient(&SentryConfig{}, "tok"), ts.URL) + res, _ := c.TestAuth(context.Background()) + if res.OK || !strings.Contains(res.Error, "instance URL") { + t.Errorf("expected instance-URL hint for 502, got %+v", res) + } + }) + t.Run("unreachable host", func(t *testing.T) { + s := httptest.NewServer(http.HandlerFunc(func(http.ResponseWriter, *http.Request) {})) + dead := s.URL + s.Close() // close so the next request is refused immediately + c := pointTo(NewRESTClient(&SentryConfig{}, "tok"), dead) + res, err := c.TestAuth(context.Background()) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + if res.OK || !strings.Contains(res.Error, "instance URL") { + t.Errorf("expected instance-URL hint for unreachable host, got %+v", res) + } + }) +} + +// TestRESTClient_TestAuth_Unauthorized_BlamesToken complements the wrong-URL +// cases: a 401/403 must read as an auth-token problem, not a URL problem. +func TestRESTClient_TestAuth_Unauthorized_BlamesToken(t *testing.T) { + ts := newMockServer(t, func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusUnauthorized) + _, _ = w.Write([]byte(`{"detail":"Invalid token"}`)) + }) + c := pointTo(NewRESTClient(&SentryConfig{}, "bad"), ts.URL) + res, _ := c.TestAuth(context.Background()) + if res.OK { + t.Fatal("expected OK=false") + } + if strings.Contains(res.Error, "instance URL") { + t.Errorf("401 should blame the token, not the URL: %q", res.Error) + } + if !strings.Contains(res.Error, "token") { + t.Errorf("expected token hint, got %q", res.Error) + } +} diff --git a/apps/backend/internal/sentry/service.go b/apps/backend/internal/sentry/service.go index dd41c6910b..668813cbd1 100644 --- a/apps/backend/internal/sentry/service.go +++ b/apps/backend/internal/sentry/service.go @@ -4,6 +4,8 @@ import ( "context" "errors" "fmt" + "net/url" + "strings" "sync" "time" @@ -102,7 +104,7 @@ func (s *Service) SetConfig(ctx context.Context, req *SetConfigRequest) (*Sentry if err := validateConfigRequest(req); err != nil { return nil, fmt.Errorf("%w: %s", ErrInvalidConfig, err.Error()) } - cfg := &SentryConfig{AuthMethod: req.AuthMethod} + cfg := &SentryConfig{AuthMethod: req.AuthMethod, URL: req.URL} if err := s.store.UpsertConfig(ctx, cfg); err != nil { return nil, fmt.Errorf("upsert sentry config: %w", err) } @@ -296,8 +298,11 @@ func (s *Service) invalidateClient() { // resolveCredentials picks credentials for a test: inline if the request // carries a secret, otherwise the stored secret. func (s *Service) resolveCredentials(ctx context.Context, req *SetConfigRequest) (*SentryConfig, string, error) { - cfg := &SentryConfig{AuthMethod: req.AuthMethod} + cfg := &SentryConfig{AuthMethod: req.AuthMethod, URL: normalizeSentryURL(req.URL)} if req.Secret != "" { + if cfg.URL == "" { + cfg.URL = DefaultSentryURL + } return cfg, req.Secret, nil } if s.secrets == nil { @@ -315,12 +320,23 @@ func (s *Service) resolveCredentials(ctx context.Context, req *SetConfigRequest) if storeErr != nil { s.log.Warn("sentry: load stored config for credential resolution failed", zap.Error(storeErr)) } - if stored != nil && cfg.AuthMethod == "" { - cfg.AuthMethod = stored.AuthMethod + if stored != nil { + if cfg.AuthMethod == "" { + cfg.AuthMethod = stored.AuthMethod + } + if cfg.URL == "" { + cfg.URL = stored.URL + } + } + if cfg.URL == "" { + cfg.URL = DefaultSentryURL } return cfg, secret, nil } +// validateConfigRequest normalizes and validates a config request in place: it +// defaults a blank auth method and instance URL, rejects unknown auth methods, +// and rejects URLs the HTTP client could not use (see validateSentryURL). func validateConfigRequest(req *SetConfigRequest) error { if req.AuthMethod == "" { req.AuthMethod = AuthMethodAuthToken @@ -328,5 +344,50 @@ func validateConfigRequest(req *SetConfigRequest) error { if req.AuthMethod != AuthMethodAuthToken { return fmt.Errorf("unknown auth method: %q", req.AuthMethod) } + req.URL = normalizeSentryURL(req.URL) + if req.URL == "" { + req.URL = DefaultSentryURL + } + return validateSentryURL(req.URL) +} + +// normalizeSentryURL trims whitespace and trailing slashes and prepends +// https:// when the user typed only a host (e.g. "sentry.acme.com"). Without a +// scheme the Go HTTP client fails with "unsupported protocol scheme". An empty +// input is returned as-is so callers can apply the sentry.io default. +func normalizeSentryURL(raw string) string { + s := strings.TrimSpace(raw) + s = strings.TrimRight(s, "/") + if s == "" { + return s + } + if !strings.Contains(s, "://") { + s = "https://" + s + } + return s +} + +// validateSentryURL rejects an instance URL the HTTP client could not use: it +// must parse, carry an http/https scheme, name a host, and be a bare host root. +// A path/query/fragment is rejected because apiPathPrefix ("/api/0") is later +// appended by string concatenation — e.g. "https://host?x=1" would otherwise +// become the malformed base "https://host?x=1/api/0". +func validateSentryURL(raw string) error { + u, err := url.Parse(raw) + if err != nil { + return fmt.Errorf("invalid instance URL: %s", err.Error()) + } + if u.Scheme != "http" && u.Scheme != "https" { + return fmt.Errorf("instance URL must use http or https: %q", raw) + } + if u.Host == "" { + return fmt.Errorf("instance URL must include a host: %q", raw) + } + if u.Path != "" && u.Path != "/" { + return fmt.Errorf("instance URL must be a host root without a path: %q", raw) + } + if u.RawQuery != "" || u.Fragment != "" { + return fmt.Errorf("instance URL must not include a query or fragment: %q", raw) + } return nil } diff --git a/apps/backend/internal/sentry/service_test.go b/apps/backend/internal/sentry/service_test.go index 828816005b..48cec91208 100644 --- a/apps/backend/internal/sentry/service_test.go +++ b/apps/backend/internal/sentry/service_test.go @@ -413,3 +413,130 @@ func TestService_ClientFor_InvalidateDuringBuild(t *testing.T) { t.Errorf("expected factory called at least twice (once per clientFor after invalidation), got %d", got) } } + +// TestService_SetConfig_DefaultsURLToSentryIO asserts a blank instance URL is +// stored as the sentry.io SaaS default. +func TestService_SetConfig_DefaultsURLToSentryIO(t *testing.T) { + f := newSvcFixture(t) + cfg, err := f.svc.SetConfig(context.Background(), &SetConfigRequest{ + AuthMethod: AuthMethodAuthToken, Secret: "tok", + }) + if err != nil { + t.Fatalf("set: %v", err) + } + if cfg.URL != DefaultSentryURL { + t.Errorf("expected URL defaulted to %q, got %q", DefaultSentryURL, cfg.URL) + } +} + +// TestService_SetConfig_NormalizesAndPersistsURL asserts a host-only URL is +// normalized (scheme added, trailing slash trimmed) before being stored. +func TestService_SetConfig_NormalizesAndPersistsURL(t *testing.T) { + f := newSvcFixture(t) + cfg, err := f.svc.SetConfig(context.Background(), &SetConfigRequest{ + AuthMethod: AuthMethodAuthToken, Secret: "tok", URL: "sentry.example.com/", + }) + if err != nil { + t.Fatalf("set: %v", err) + } + if cfg.URL != "https://sentry.example.com" { + t.Errorf("expected normalized URL, got %q", cfg.URL) + } +} + +// TestService_SetConfig_RejectsMalformedURL asserts a non-http(s) URL is +// rejected at save time. +func TestService_SetConfig_RejectsMalformedURL(t *testing.T) { + f := newSvcFixture(t) + _, err := f.svc.SetConfig(context.Background(), &SetConfigRequest{ + AuthMethod: AuthMethodAuthToken, Secret: "tok", URL: "ftp://nope", + }) + if err == nil { + t.Error("expected validation error for non-http(s) URL") + } +} + +// TestService_TestConnection_PassesConfiguredURL ensures a pre-save test uses +// the URL the user typed in the form, so a self-hosted instance can be +// validated before the config is persisted. +func TestService_TestConnection_PassesConfiguredURL(t *testing.T) { + f := newSvcFixture(t) + var sawURL string + f.svc.clientFn = func(cfg *SentryConfig, _ string) Client { + sawURL = cfg.URL + return f.client + } + if _, err := f.svc.TestConnection(context.Background(), &SetConfigRequest{ + AuthMethod: AuthMethodAuthToken, Secret: "inline", URL: "https://sentry.example.com", + }); err != nil { + t.Fatalf("test: %v", err) + } + if sawURL != "https://sentry.example.com" { + t.Errorf("expected configured URL passed to client, got %q", sawURL) + } +} + +// TestService_TestConnection_FallsBackToStoredURL covers the post-save "Test +// connection" path (blank secret, blank URL in the request): the stored +// instance URL must be used rather than silently reverting to sentry.io. +func TestService_TestConnection_FallsBackToStoredURL(t *testing.T) { + f := newSvcFixture(t) + ctx := context.Background() + if _, err := f.svc.SetConfig(ctx, &SetConfigRequest{ + AuthMethod: AuthMethodAuthToken, Secret: "stored", URL: "https://sentry.example.com", + }); err != nil { + t.Fatalf("set: %v", err) + } + waitForAuthProbe(t, f) + var sawURL string + f.svc.clientFn = func(cfg *SentryConfig, _ string) Client { + sawURL = cfg.URL + return f.client + } + if _, err := f.svc.TestConnection(ctx, &SetConfigRequest{AuthMethod: AuthMethodAuthToken}); err != nil { + t.Fatalf("test: %v", err) + } + if sawURL != "https://sentry.example.com" { + t.Errorf("expected stored URL used, got %q", sawURL) + } +} + +// TestService_SetConfig_RejectsNonRootURL asserts URLs carrying a path, query, +// or fragment are rejected, so /api/0 cannot be appended to a malformed base. +func TestService_SetConfig_RejectsNonRootURL(t *testing.T) { + cases := []struct { + name string + url string + }{ + {"path", "https://sentry.example.com/some/path"}, + {"query", "https://sentry.example.com?x=1"}, + {"fragment", "https://sentry.example.com#frag"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + f := newSvcFixture(t) + _, err := f.svc.SetConfig(context.Background(), &SetConfigRequest{ + AuthMethod: AuthMethodAuthToken, Secret: "tok", URL: tc.url, + }) + if err == nil { + t.Errorf("expected rejection of non-root URL %q", tc.url) + } + }) + } +} + +// TestService_SetConfig_AllowsHostRootWithTrailingSlash guards the boundary: +// a bare host root (with or without a trailing slash) must still be accepted +// and normalized, so the non-root rejection above doesn't over-reach. +func TestService_SetConfig_AllowsHostRootWithTrailingSlash(t *testing.T) { + f := newSvcFixture(t) + cfg, err := f.svc.SetConfig(context.Background(), &SetConfigRequest{ + AuthMethod: AuthMethodAuthToken, Secret: "tok", URL: "https://sentry.example.com/", + }) + if err != nil { + t.Fatalf("host root with trailing slash should be accepted: %v", err) + } + if cfg.URL != "https://sentry.example.com" { + t.Errorf("expected trailing slash trimmed, got %q", cfg.URL) + } +} diff --git a/apps/backend/internal/sentry/store.go b/apps/backend/internal/sentry/store.go index f5fcad384e..ef6c8639c6 100644 --- a/apps/backend/internal/sentry/store.go +++ b/apps/backend/internal/sentry/store.go @@ -30,6 +30,7 @@ const createTablesSQL = ` CREATE TABLE IF NOT EXISTS sentry_configs ( id TEXT PRIMARY KEY CHECK(id = 'singleton'), auth_method TEXT NOT NULL, + url TEXT NOT NULL DEFAULT 'https://sentry.io', last_checked_at DATETIME, last_ok INTEGER NOT NULL DEFAULT 0, last_error TEXT NOT NULL DEFAULT '', @@ -73,16 +74,43 @@ const createTablesSQL = ` // singletonID is the synthetic primary key of the (only) row in sentry_configs. const singletonID = "singleton" +// initSchema creates the integration tables when absent and applies the +// additive column migrations that bring older databases to the current schema. func (s *Store) initSchema() error { if _, err := s.db.Exec(createTablesSQL); err != nil { return err } + if err := s.addConfigURLColumn(); err != nil { + return err + } if err := s.addMaxInflightTasksColumn(); err != nil { return err } return s.addIssueWatchLastErrorColumns() } +// addConfigURLColumn brings older databases up to the current schema by adding +// the url column to sentry_configs when missing. Existing rows — all SaaS +// installs, since this predates self-hosted support — backfill to the +// sentry.io default (mirrors DefaultSentryURL). A fresh install hits the +// column-already-present branch since createTablesSQL declares the column. +func (s *Store) addConfigURLColumn() error { + cols, err := s.tableColumns("sentry_configs") + if err != nil { + return err + } + if len(cols) == 0 { + return nil + } + if _, ok := cols["url"]; ok { + return nil + } + if _, err := s.db.Exec(`ALTER TABLE sentry_configs ADD COLUMN url TEXT NOT NULL DEFAULT 'https://sentry.io'`); err != nil { + return fmt.Errorf("add url column: %w", err) + } + return nil +} + // addMaxInflightTasksColumn brings older databases up to the current schema by // adding the max_inflight_tasks column to sentry_issue_watches when missing. // Existing rows backfill to the default (5). A fresh install hits the @@ -156,7 +184,7 @@ func (s *Store) tableColumns(table string) (map[string]struct{}, error) { return cols, rows.Err() } -const selectConfigColumns = `auth_method, +const selectConfigColumns = `auth_method, url, last_checked_at, last_ok, last_error, created_at, updated_at` // GetConfig returns the singleton Sentry config, or nil when no row exists. @@ -182,12 +210,13 @@ func (s *Store) UpsertConfig(ctx context.Context, cfg *SentryConfig) error { } cfg.UpdatedAt = now _, err := s.db.ExecContext(ctx, ` - INSERT INTO sentry_configs (id, auth_method, created_at, updated_at) - VALUES (?, ?, ?, ?) + INSERT INTO sentry_configs (id, auth_method, url, created_at, updated_at) + VALUES (?, ?, ?, ?, ?) ON CONFLICT(id) DO UPDATE SET auth_method = excluded.auth_method, + url = excluded.url, updated_at = excluded.updated_at`, - singletonID, cfg.AuthMethod, cfg.CreatedAt, cfg.UpdatedAt) + singletonID, cfg.AuthMethod, cfg.URL, cfg.CreatedAt, cfg.UpdatedAt) return err } diff --git a/apps/backend/internal/sentry/store_test.go b/apps/backend/internal/sentry/store_test.go index edb6c9873e..028a768d19 100644 --- a/apps/backend/internal/sentry/store_test.go +++ b/apps/backend/internal/sentry/store_test.go @@ -130,3 +130,65 @@ func TestStore_UpdateAuthHealth(t *testing.T) { t.Errorf("expected last_error preserved, got %q", cfg.LastError) } } + +// TestStore_UpsertGetConfig_RoundTripsURL asserts the instance URL persists +// through an upsert/get cycle. +func TestStore_UpsertGetConfig_RoundTripsURL(t *testing.T) { + store := newTestStore(t) + ctx := context.Background() + cfg := &SentryConfig{AuthMethod: AuthMethodAuthToken, URL: "https://sentry.example.com"} + if err := store.UpsertConfig(ctx, cfg); err != nil { + t.Fatalf("upsert: %v", err) + } + got, err := store.GetConfig(ctx) + if err != nil || got == nil { + t.Fatalf("get: %v / %v", err, got) + } + if got.URL != "https://sentry.example.com" { + t.Errorf("url not persisted: got %q", got.URL) + } +} + +// TestStore_MigratesURLColumn seeds a pre-self-hosted sentry_configs table (no +// url column) and verifies NewStore adds the column, backfilling the existing +// SaaS row to the sentry.io default rather than an empty string. +func TestStore_MigratesURLColumn(t *testing.T) { + raw, err := sql.Open("sqlite3", ":memory:") + if err != nil { + t.Fatalf("open sqlite: %v", err) + } + raw.SetMaxOpenConns(1) + raw.SetMaxIdleConns(1) + db := sqlx.NewDb(raw, "sqlite3") + t.Cleanup(func() { _ = db.Close() }) + + now := time.Now().UTC() + if _, err := db.Exec(` + CREATE TABLE sentry_configs ( + id TEXT PRIMARY KEY CHECK(id = 'singleton'), + auth_method TEXT NOT NULL, + last_checked_at DATETIME, + last_ok INTEGER NOT NULL DEFAULT 0, + last_error TEXT NOT NULL DEFAULT '', + created_at DATETIME NOT NULL, + updated_at DATETIME NOT NULL + )`); err != nil { + t.Fatalf("create legacy schema: %v", err) + } + if _, err := db.Exec(`INSERT INTO sentry_configs (id, auth_method, created_at, updated_at) + VALUES ('singleton', ?, ?, ?)`, AuthMethodAuthToken, now, now); err != nil { + t.Fatalf("seed legacy row: %v", err) + } + + store, err := NewStore(db, db) + if err != nil { + t.Fatalf("new store (migrate): %v", err) + } + cfg, err := store.GetConfig(context.Background()) + if err != nil || cfg == nil { + t.Fatalf("get after migrate: %v / %v", err, cfg) + } + if cfg.URL != DefaultSentryURL { + t.Errorf("expected legacy row backfilled to %q, got %q", DefaultSentryURL, cfg.URL) + } +} diff --git a/apps/web/components/sentry/sentry-settings.tsx b/apps/web/components/sentry/sentry-settings.tsx index 8c438451ec..f28cf5cdef 100644 --- a/apps/web/components/sentry/sentry-settings.tsx +++ b/apps/web/components/sentry/sentry-settings.tsx @@ -26,21 +26,23 @@ import { } from "@/lib/api/domains/sentry-api"; import { SENTRY_AUTH_METHOD, + SENTRY_DEFAULT_URL, type SentryConfig, type TestSentryConnectionResult, } from "@/lib/types/sentry"; import { SentryIssueWatchersSection } from "./sentry-issue-watchers-section"; type FormState = { + url: string; secret: string; }; -const emptyForm: FormState = { secret: "" }; +const emptyForm: FormState = { url: SENTRY_DEFAULT_URL, secret: "" }; -function configToForm(_cfg: SentryConfig | null): FormState { - // Only the (write-only) secret is editable here; org/project are chosen - // per-watcher and per-browse, not stored install-wide. - return { secret: "" }; +function configToForm(cfg: SentryConfig | null): FormState { + // Only the instance URL and the (write-only) secret are editable here; + // org/project are chosen per-watcher and per-browse, not stored install-wide. + return { url: cfg?.url || SENTRY_DEFAULT_URL, secret: "" }; } function saveLabel(saving: boolean, hasConfig: boolean): string { @@ -60,6 +62,33 @@ function configToHealth(config: SentryConfig | null): IntegrationAuthHealth | nu type UpdateFn = (key: K, value: FormState[K]) => void; +type UrlFieldProps = { + form: FormState; + loading: boolean; + update: UpdateFn; +}; + +function UrlField({ form, loading, update }: UrlFieldProps) { + return ( +
+ + update("url", e.target.value)} + disabled={loading} + /> +

+ Base URL of your Sentry instance. Leave as {SENTRY_DEFAULT_URL} for Sentry SaaS, or point it + at a self-hosted install (e.g. https://sentry.your-company.com). +

+
+ ); +} + type SecretFieldProps = { form: FormState; loading: boolean; @@ -220,7 +249,7 @@ function useSettingsActions({ form, setConfig, setForm, setTestResult }: Setting setTesting(true); setTestResult(null); try { - const res = await testSentryConnection(form.secret || undefined); + const res = await testSentryConnection(form.secret || undefined, form.url || undefined); setTestResult(res); } catch (err) { setTestResult({ ok: false, error: String(err) }); @@ -234,6 +263,7 @@ function useSettingsActions({ form, setConfig, setForm, setTestResult }: Setting try { const saved = await saveSentryConfig({ authMethod: SENTRY_AUTH_METHOD, + url: form.url, secret: form.secret, }); setConfig(saved); @@ -360,6 +390,7 @@ export function SentryConnectionSection() { + { await expect(settings.saveButton).toHaveText(/Update/i); await expect(settings.deleteButton).toBeVisible(); }); + + // Self-hosted support: the instance URL defaults to sentry.io, accepts a + // custom host, and persists across reload via the saved config. + test("defaults the instance URL and persists a self-hosted URL", async ({ + testPage, + apiClient, + }) => { + await apiClient.mockSentryReset(); + await apiClient.mockSentrySetAuthResult({ + ok: true, + userId: "u-2", + displayName: "Self Hosted", + }); + + const settings = new SentrySettingsPage(testPage); + await settings.goto(); + + // The URL field is pre-filled with the SaaS default. + await expect(settings.urlInput).toHaveValue("https://sentry.io"); + + // Point it at a self-hosted instance and save with a token. + await settings.urlInput.fill("https://sentry.acme.example.com"); + await settings.secretInput.fill(TOKEN); + await settings.saveButton.click(); + await expect(settings.saveButton).toHaveText(/Update/i); + await apiClient.waitForIntegrationAuthHealthy("sentry"); + + // Reload: the custom instance URL round-trips through the saved config. + await settings.goto(); + await expect(settings.urlInput).toHaveValue("https://sentry.acme.example.com"); + }); }); diff --git a/apps/web/lib/api/domains/sentry-api.ts b/apps/web/lib/api/domains/sentry-api.ts index f6b3e85216..e7c5d86cb8 100644 --- a/apps/web/lib/api/domains/sentry-api.ts +++ b/apps/web/lib/api/domains/sentry-api.ts @@ -37,13 +37,20 @@ export async function deleteSentryConfig(options?: ApiRequestOptions) { }); } -export async function testSentryConnection(secret?: string, options?: ApiRequestOptions) { +export async function testSentryConnection( + secret?: string, + url?: string, + options?: ApiRequestOptions, +) { + const payload: { secret?: string; url?: string } = {}; + if (secret) payload.secret = secret; + if (url) payload.url = url; return fetchJson(`/api/v1/sentry/config/test`, { ...options, init: { ...(options?.init ?? {}), method: "POST", - body: JSON.stringify(secret ? { secret } : {}), + body: JSON.stringify(payload), }, }); } diff --git a/apps/web/lib/types/sentry.ts b/apps/web/lib/types/sentry.ts index 44c8b1b2dd..46325adbfa 100644 --- a/apps/web/lib/types/sentry.ts +++ b/apps/web/lib/types/sentry.ts @@ -2,8 +2,14 @@ export type SentryAuthMethod = "auth_token"; export const SENTRY_AUTH_METHOD: SentryAuthMethod = "auth_token"; +// SENTRY_DEFAULT_URL is the SaaS base URL pre-filled in the settings form and +// used by the backend when no custom instance URL is configured. Self-hosted +// installs replace it with their own host. +export const SENTRY_DEFAULT_URL = "https://sentry.io"; + export interface SentryConfig { authMethod: SentryAuthMethod; + url: string; hasSecret: boolean; lastCheckedAt?: string | null; lastOk: boolean; @@ -14,6 +20,7 @@ export interface SentryConfig { export interface SetSentryConfigRequest { authMethod: SentryAuthMethod; + url: string; secret: string; }