diff --git a/README.md b/README.md index 836ac01..4519294 100644 --- a/README.md +++ b/README.md @@ -161,6 +161,8 @@ POST /v1/api-keys (auth required, raw key returned once) GET /v1/api-keys (auth required) DELETE /v1/api-keys/{keyID} (auth required) +POST /v1/ask-ai (auth required, NOT admin — see below) + GET /v1/admin/oauth/health (admin required) GET /v1/admin/security/hash-migration (admin required) GET /v1/admin/security/mfa-adoption (admin required) @@ -481,7 +483,26 @@ The LLM API key and the database connection string are sealed with **AES-256-GCM - **`"*"` as an origin is refused by name**, with the reason in the message: this widget answers questions about the signed-in user's sessions and audit events, so a wildcard origin would let any page on the internet ask them through a visitor's browser. - **The refusal names neither the entity nor the scope.** That error reaches an end user through the widget; listing the configured entities would be describing the console's schema to whoever is typing questions at it. - **A disabled widget may be otherwise empty**, so switching the feature off does not require filling in fields that are about to stop mattering. Anything that *is* filled in is still validated, so a form cannot store a value that was never checked and would be rejected the moment it was switched on. -- **No embed snippet is returned.** The snippet is markup the console renders into its own pages, and the URL in it would name an endpoint this API does not serve yet — returning one would hand the console a script tag pointing at a 404. What this endpoint owes the console is the configuration a snippet is built from. +- **No embed snippet is returned.** The snippet is markup the console renders into its own pages, and this repo has no opinion on another repo's markup. An earlier version of this note gave a different reason — that the URL in a snippet would name an endpoint that did not exist — and that expired in spec 1.7, when `POST /v1/ask-ai` started serving the widget. What this endpoint owes the console is the configuration a snippet is built from. + +### Serving the widget + +`POST /v1/ask-ai` is where the widget's questions are answered, and it is the **one AI-assisted endpoint here that is not behind `RequireAdmin`**. That is the feature, not an oversight: the widget belongs to an end user and answers questions about *their own* account — the sessions and audit events on their own row. An operator is also a user of their own account, so an operator's token works too, but it works as an account holder, not as an operator. + +```json +// POST /v1/ask-ai Authorization: Bearer +{ "question": "when did I last log in" } +// 200 +{ "data": { "answer": "id | created_at\ns-2 | 2026-09-17T09:12:44Z", "row_count": 1 } } +``` + +- **The caller's identity is the token and nothing else.** The body has no field that can name a user; one sent anyway is ignored rather than rejected, because there is nothing for it to reach. Two independent things hold that: `widget.Ask` discards whatever identity filter the model produced and substitutes the id this repo verified from the token, and the id itself comes from `RequireAuth` rather than from anything the request can carry. A body that tries to name someone else is the case the tests pin. +- **`answer` is rendered, not composed.** `widget.Config.Composer` is left nil deliberately, so cryden falls back to `RenderResult` — a deterministic plain-text table over rows that have already been scoped to this one user. Supplying a composer would mean a second model call per question whose output nothing validates, to turn a table into prose. That is worth doing deliberately if it is ever wanted; it is not a default to fall into. +- **`allowed_origins` is defense in depth, not the boundary.** The Bearer token is the boundary and it is verified first. `Origin` is a header a browser sets and anything that is not a browser sets freely, which is why a request carrying *no* Origin is allowed: refusing it would break every non-browser client — the console, a mobile app, a test — while stopping nobody, since whoever can forge an allowlisted origin can also omit it. What the check does buy is a guard against a stray embed on a site nobody meant to authorise, which would otherwise put one user's answers in front of whoever is browsing that page. Origins are compared on scheme and host with the default port normalised away, so `https://console.example.com:443` and `https://console.example.com` are the same entry rather than one that silently never matches. +- **Settings are read per question, not cached behind an invalidation.** Three short reads and two AES-GCM opens against one model call that takes hundreds of milliseconds at best — the trade is lopsided, and the failure mode of the other side is a saved settings change that does not take effect until a restart, or takes effect only if every future writer remembers to poke a hook. What *is* cached is the built provider pair, keyed on a SHA-256 digest of the settings it was built from. Hashed rather than held, because two of the three inputs are credentials and a cache key lives as long as the process does. +- **Building the provider is cheap on purpose.** `aiprovider.NewPostgresSnapshot` uses `sql.Open`, which does not connect, so a rebuild is an allocation rather than a round trip and a stored connection that has since gone bad is reported by the query that uses it. That is what keeps a broken setting from being a startup failure. +- **Switched off and never configured are one answer** — `404 ask_ai_widget_disabled`. The stored config's zero value is disabled, so there is no way to tell them apart, and there is no reason a caller should care. `404 not_configured` is the different case: the widget is on but the deployment has no LLM provider or no read-only database stored. +- **It costs money per question and is not rate-limited per user.** It is bounded only by `EDGE_RATE_LIMIT`, which counts requests per edge, not per account. A real per-user limit needs a policy this repo has not decided — the numbers are deployment-specific — so it is recorded as owed in `PROGRESS.md` rather than invented here. ## Design notes diff --git a/askai/askai.go b/askai/askai.go new file mode 100644 index 0000000..8134ccc --- /dev/null +++ b/askai/askai.go @@ -0,0 +1,457 @@ +// Package askai is the serving side of the ask-ai widget: cryden's +// owner-scoped query surface, handed to the signed-in end user it +// belongs to. +// +// The security boundary is not in this package. cryden's widget.Ask +// force-scopes every parsed intent to the one identity the caller +// supplies — it discards whatever identity filter the model produced +// and substitutes the real one — and aiprovider.ScopedProvider narrows +// which entities a deployment is willing to answer over at all. What +// this package adds is the last two things neither of those can do: the +// owner id must come from this repo's own authentication of the +// request, and the two providers cryden's widget.Config needs must be +// built from what an operator stored in the settings table. +// +// See docs/design of cryden's widget package for why ownerUserID is +// never read from the question or from anything derived from it. +package askai + +import ( + "context" + "crypto/sha256" + "encoding/hex" + "errors" + "fmt" + "io" + "net/url" + "strings" + "sync" + + crydenai "github.com/crydensync/cryden/v2/ai" + "github.com/crydensync/cryden/v2/widget" + + "github.com/crydensync/api/aiprovider" + "github.com/crydensync/api/settings" +) + +// maxQuestionLength bounds one question. This is chat copy typed into a +// widget, not a document: the bound is on what the deployment will pay +// to have parsed, since every question is at least one model call. +const maxQuestionLength = 500 + +var ( + // ErrNotConfigured means the widget is switched on but the + // deployment has no LLM provider or no read-only database stored, + // so there is nothing to answer with. A wiring fact, not a fault — + // mapped to 404 like every other unconfigured feature in this api. + ErrNotConfigured = errors.New("askai: the widget has no LLM provider or database configured") + + // ErrWidgetDisabled means an operator has switched the widget off. + // This is the answer whether the setting was stored with + // enabled=false or never stored at all: an unconfigured widget and + // a deliberately disabled one are the same thing to a caller, and + // the zero value of the stored config is disabled. + ErrWidgetDisabled = errors.New("askai: the widget is switched off on this deployment") + + // ErrOriginNotAllowed means the request carried an Origin header + // that is not in the configured allowlist. See checkOrigin for what + // this does and does not protect against. + ErrOriginNotAllowed = errors.New("askai: that origin is not allowed to embed the widget") + + // ErrInvalidQuestion means the question was empty or longer than + // maxQuestionLength. Both are the caller's input rather than a + // deployment fault, so both are one 400 with the reason in the + // message rather than two codes nothing would branch on. + ErrInvalidQuestion = errors.New("askai: invalid question") +) + +// Service answers widget questions. It holds no state of its own beyond +// a cache of the providers built from the current settings. +type Service struct { + secrets *settings.Secrets + + // providers builds the engine-facing pair from a stored + // configuration. Nil means the real ones — see defaultProviders. + // + // It exists so that this package's own behaviour is testable without a + // database: everything below the seam is cryden's widget.Ask and a + // SELECT, and everything above it is this repo's. Leaving it nil in + // production is the point — the concrete constructors are what ship — + // and a test in this package replaces it with doubles so it can see + // the intent that actually reached the store, which is the only way to + // check that a question about somebody else cannot be asked. + providers Providers + + // mu guards current. Held across a rebuild, which is cheap by + // design — see build. + mu sync.Mutex + current *built +} + +// Providers builds the pair cryden's widget.Config needs from a stored +// configuration. +// +// It returns the raw provider and store; wrapping the provider in the +// scope check is build's job, not the factory's, so replacing this still +// exercises the real scope enforcement. +// +// The default is the Anthropic provider over the stored credential and +// the Postgres snapshot over the stored read-only connection. A host +// that wants a different LLM backend supplies its own, and so does a +// test that needs to see what reached the query surface. +type Providers func( + llm settings.LLMProviderConfig, + db settings.DatabaseProviderConfig, +) (crydenai.LLMProvider, crydenai.QueryableStore, io.Closer, error) + +// defaultProviders builds what production runs: the Anthropic provider +// over the stored credential, and the snapshot over the stored read-only +// connection. +func defaultProviders(llm settings.LLMProviderConfig, db settings.DatabaseProviderConfig) (crydenai.LLMProvider, crydenai.QueryableStore, io.Closer, error) { + provider, err := aiprovider.NewAnthropic(aiprovider.AnthropicConfig{ + APIKey: llm.APIKey, + Model: llm.Model, + MaxTokens: llm.MaxTokens, + }) + if err != nil { + return nil, nil, nil, err + } + + snapshot, err := aiprovider.NewPostgresSnapshot(db.DSN, db.MaxRows) + if err != nil { + return nil, nil, nil, err + } + return provider, snapshot, snapshot, nil +} + +// built is the pair cryden's widget.Config needs, plus what it takes to +// tell whether it is still the pair the stored settings describe. +type built struct { + // fingerprint is a digest of the settings this was built from, not + // the settings themselves. Held as a digest because two of the three + // inputs are credentials and a cache key is no place for one. + fingerprint string + + provider crydenai.LLMProvider + store crydenai.QueryableStore + + // closer releases whatever the factory opened — the snapshot's pool, + // in production. An interface rather than the concrete type because + // the QueryableStore contract has no Close on it, and because a test's + // double needs somewhere to hang its own cleanup. + closer io.Closer +} + +// New returns a Service over the settings store. A Service built over a +// Secrets with no encryption key is valid and answers ErrNotConfigured: +// the settings endpoints themselves are 404 in that deployment, so there +// is nothing that could have been configured. +func New(secrets *settings.Secrets) *Service { + return &Service{secrets: secrets, providers: defaultProviders} +} + +// NewWithProviders returns a Service that builds its providers with +// build rather than the default. See Providers — this is the hook for a +// host running a different LLM backend, and for a test that needs to +// observe what reaches the query surface. +func NewWithProviders(secrets *settings.Secrets, build Providers) *Service { + return &Service{secrets: secrets, providers: build} +} + +// Request is one widget question. +type Request struct { + // OwnerUserID is the authenticated caller, from this repo's own + // verification of their token. Never from the body and never from + // anything the model produced — see the package doc. + OwnerUserID string + + Question string + + // Origin is the request's Origin header, empty when it carried + // none. Only ever consulted against the configured allowlist. + Origin string +} + +// Ask answers req.Question on behalf of req.OwnerUserID. +func (s *Service) Ask(ctx context.Context, req Request) (widget.Answer, error) { + // Checked here as well as in the handler. The handler refuses an + // empty question with a message written for a person; this is the + // guard that holds for any other caller, because an empty question + // reaching the model is a model call that buys nothing. + if req.OwnerUserID == "" { + return widget.Answer{}, widget.ErrMissingOwner + } + if strings.TrimSpace(req.Question) == "" { + return widget.Answer{}, fmt.Errorf("%w: a question is required", ErrInvalidQuestion) + } + if len(req.Question) > maxQuestionLength { + return widget.Answer{}, fmt.Errorf("%w: a question is limited to %d characters, got %d", + ErrInvalidQuestion, maxQuestionLength, len(req.Question)) + } + + config, err := s.widgetConfig(ctx) + if err != nil { + return widget.Answer{}, err + } + // Enabled is checked before the origin, so a deployment that has + // switched the widget off answers "off" rather than "that origin is + // not allowed" — the second would be true and misleading, naming a + // configuration detail of a feature that is not running. + if !config.Enabled { + return widget.Answer{}, ErrWidgetDisabled + } + if err := checkOrigin(config.AllowedOrigins, req.Origin); err != nil { + return widget.Answer{}, err + } + + // Read on every question rather than cached behind an invalidation + // the settings handlers would have to remember to fire. Three short + // reads and two AES-GCM opens, against one model call that takes + // hundreds of milliseconds at best: the wrong side of this trade is + // the one where a saved settings change does not take effect until a + // restart, or takes effect only if every future writer remembers to + // poke a hook. + llmRaw, err := s.secrets.Get(ctx, settings.KeyLLMProvider) + if errors.Is(err, settings.ErrNotFound) { + return widget.Answer{}, ErrNotConfigured + } + if err != nil { + return widget.Answer{}, err + } + dbRaw, err := s.secrets.Get(ctx, settings.KeyDatabaseProvider) + if errors.Is(err, settings.ErrNotFound) { + return widget.Answer{}, ErrNotConfigured + } + if err != nil { + return widget.Answer{}, err + } + + ready, err := s.build(llmRaw, dbRaw, config.Entities) + if err != nil { + return widget.Answer{}, err + } + + // No Composer. cryden's widget package documents a nil Composer as + // a valid, strictly safer default, and falls back to RenderResult — + // a deterministic plain-text table built from rows that have already + // been scoped to this one user. Supplying one would mean a second + // model call per question, whose output nothing validates, for + // prose. That is a decision worth making deliberately if it is ever + // wanted; it is not one to make by default. + return widget.Ask(ctx, widget.Config{ + Provider: ready.provider, + Store: ready.store, + }, req.OwnerUserID, req.Question) +} + +// Close releases the connection pool behind the cached providers. The +// cached pair is replaced and closed on every rebuild, so this is only +// about the last one — and nothing calls it yet, because this repo still +// has no graceful shutdown for it to hang off. It exists so that adding +// one does not have to start by widening this type's API. +func (s *Service) Close() error { + s.mu.Lock() + defer s.mu.Unlock() + if s.current == nil { + return nil + } + var err error + if s.current.closer != nil { + err = s.current.closer.Close() + } + s.current = nil + return err +} + +// widgetConfig reads the stored widget configuration. Nothing stored +// reads as the zero value, which is disabled — the same answer an +// operator gets from GET /v1/admin/settings/ask-ai-widget, and the +// reason this returns no error for that case. +func (s *Service) widgetConfig(ctx context.Context) (settings.AskAIWidgetConfig, error) { + raw, err := s.secrets.Get(ctx, settings.KeyAskAIWidget) + if errors.Is(err, settings.ErrNotFound) { + return settings.AskAIWidgetConfig{}, nil + } + if err != nil { + return settings.AskAIWidgetConfig{}, err + } + return settings.UnmarshalAskAIWidget(raw) +} + +// build returns the providers for these settings, reusing the cached +// pair when they are the settings it was built from. +// +// The check is on content rather than on a version or a timestamp: the +// settings are read fresh on every question, so comparing what was read +// against what is cached is the whole mechanism, and there is no window +// in which a change has been saved but not noticed. +// +// Building is deliberately cheap — aiprovider.NewPostgresSnapshot uses +// sql.Open, which does not connect — so a rebuild is a struct allocation +// and not a round trip. A stored connection that has since gone bad is +// therefore reported by the query that uses it rather than by this +// function, which is what keeps a broken setting from being a startup +// failure. +func (s *Service) build(llmRaw, dbRaw []byte, entities []string) (*built, error) { + fingerprint := fingerprint(llmRaw, dbRaw, entities) + + s.mu.Lock() + defer s.mu.Unlock() + + if s.current != nil && s.current.fingerprint == fingerprint { + return s.current, nil + } + + llmConfig, err := settings.UnmarshalLLMProvider(llmRaw) + if err != nil { + return nil, err + } + dbConfig, err := settings.UnmarshalDatabaseProvider(dbRaw) + if err != nil { + return nil, err + } + + build := s.providers + if build == nil { + // The zero value builds what production builds, so a Service + // assembled as a struct literal — which a test may well do — + // behaves like one from New rather than panicking. + build = defaultProviders + } + + provider, store, closer, err := build(llmConfig, dbConfig) + if err != nil { + return nil, err + } + + next := &built{ + fingerprint: fingerprint, + // The scoped provider is what gives the widget's `entities` + // setting teeth — see aiprovider.ScopedProvider. It wraps the + // LLM provider and not the store, because the entity is the + // model's output and refusing it is only possible at parse time. + provider: aiprovider.NewScopedProvider(provider, entities), + store: store, + closer: closer, + } + + if s.current != nil { + // Closed while holding the lock, so a concurrent question cannot + // be mid-query on a pool this is tearing down. The old pair is + // only ever replaced here. + if s.current.closer != nil { + _ = s.current.closer.Close() + } + } + s.current = next + return next, nil +} + +// fingerprint digests the settings a pair of providers was built from. +// +// Hashed rather than kept as a string because the LLM provider's +// plaintext is an API key and the database's is a connection string with +// a password in it, and a cache key lives as long as the process does. +func fingerprint(llmRaw, dbRaw []byte, entities []string) string { + digest := sha256.New() + digest.Write(llmRaw) + digest.Write([]byte{0}) + digest.Write(dbRaw) + digest.Write([]byte{0}) + // Entities are part of the key because they are part of the built + // value: ScopedProvider holds them, so a scope change with no + // credential change still has to rebuild. + digest.Write([]byte(strings.Join(entities, ","))) + return hex.EncodeToString(digest.Sum(nil)) +} + +// checkOrigin refuses a request whose Origin header names an origin the +// operator has not allowed. +// +// What this is: a guard against an embed on a site nobody meant to +// authorise. It is worth having — the widget answers questions about the +// signed-in user's own sessions and audit events, and a stray embed +// elsewhere on the internet would put those answers in front of whoever +// is browsing that page. +// +// What this is not: the security boundary. The boundary is the Bearer +// token, which is verified before this runs and which no origin can +// forge. Origin is a header the browser sets and anything that is not a +// browser sets freely, so a caller who wanted past this would simply not +// send one — which is exactly why an absent Origin is allowed rather +// than refused. Refusing it would break every non-browser client (the +// console itself, a mobile app, a test) while stopping nobody: the +// attacker who can forge an allowlisted origin can also omit it. Reading +// this check as authentication is the mistake worth avoiding here. +func checkOrigin(allowed []string, origin string) error { + if strings.TrimSpace(origin) == "" { + return nil + } + for _, candidate := range allowed { + if sameOrigin(candidate, origin) { + return nil + } + } + return fmt.Errorf("%w: %q", ErrOriginNotAllowed, origin) +} + +// sameOrigin compares two origins by scheme and host, with the default +// port for the scheme treated as absent on both sides. +// +// That normalisation is not pedantry: browsers omit :443 from an https +// origin, so an operator who typed "https://console.example.com:443" +// into the allowlist would otherwise have configured an entry that could +// never match a real request — a setting that looks right, validates +// right, and silently refuses every embed. +func sameOrigin(a, b string) bool { + canonicalA, ok := canonicalOrigin(a) + if !ok { + return false + } + canonicalB, ok := canonicalOrigin(b) + if !ok { + return false + } + return canonicalA == canonicalB +} + +// canonicalOrigin reduces an origin to scheme://host[:port], and refuses +// anything that is not an origin at all. +// +// The refusals mirror settings.validateWidgetOrigin, which is the rule +// the allowlist was stored under. They have to agree: a comparison that +// silently ignored a path would accept a value the operator could never +// have stored, and the two sides of one rule disagreeing is how a check +// stops meaning what its comment says. A browser never puts a path in an +// Origin header, so nothing legitimate is refused here. +func canonicalOrigin(origin string) (string, bool) { + parsed, err := url.Parse(origin) + if err != nil { + return "", false + } + if parsed.Path != "" && parsed.Path != "/" { + return "", false + } + if parsed.RawQuery != "" || parsed.Fragment != "" || parsed.User != nil { + return "", false + } + + scheme := strings.ToLower(parsed.Scheme) + host := strings.ToLower(parsed.Hostname()) + if host == "" { + return "", false + } + // A bare host ("console.example.com") parses with an empty scheme, + // and url.Parse happily reads "https://host:443" and "host:443" as + // the same shape. Requiring the scheme is what keeps the first from + // comparing equal to an allowlisted origin. + if scheme != "http" && scheme != "https" { + return "", false + } + + port := parsed.Port() + if port == "" || (scheme == "https" && port == "443") || (scheme == "http" && port == "80") { + return scheme + "://" + host, true + } + return scheme + "://" + host + ":" + port, true +} diff --git a/askai/askai_test.go b/askai/askai_test.go new file mode 100644 index 0000000..5d075d1 --- /dev/null +++ b/askai/askai_test.go @@ -0,0 +1,699 @@ +package askai + +import ( + "context" + "errors" + "io" + "strings" + "sync" + "testing" + + crydenai "github.com/crydensync/cryden/v2/ai" + "github.com/crydensync/cryden/v2/widget" + + "github.com/crydensync/api/aiprovider" + "github.com/crydensync/api/settings" +) + +// owner is the identity this repo's authentication would have supplied. +// Every scoping assertion below is about this value reaching the store +// and no other. +const owner = "11111111-1111-1111-1111-111111111111" + +// otherUser is what the model claims. Nothing may ever run against it. +const otherUser = "99999999-9999-9999-9999-999999999999" + +const allowedOrigin = "https://console.example.com" + +// fakeProvider stands in for the LLM. It returns whatever intent the +// test says the model produced — including one that names somebody else, +// which is the case that matters. +type fakeProvider struct { + intent crydenai.QueryIntent + err error +} + +func (p *fakeProvider) ParseQueryIntent(_ context.Context, _ string) (crydenai.QueryIntent, error) { + if p.err != nil { + return crydenai.QueryIntent{}, p.err + } + return p.intent, nil +} + +// fakeStore records what actually reached the query surface. In +// production this is a Postgres snapshot on a role verified to refuse +// writes; here it is the observation point for the scope assertions. +type fakeStore struct { + mu sync.Mutex + ran []crydenai.QueryIntent + result crydenai.QueryResult + err error +} + +func (s *fakeStore) RunSafeQuery(_ context.Context, intent crydenai.QueryIntent) (crydenai.QueryResult, error) { + s.mu.Lock() + defer s.mu.Unlock() + s.ran = append(s.ran, intent) + if s.err != nil { + return crydenai.QueryResult{}, s.err + } + return s.result, nil +} + +func (s *fakeStore) last(t *testing.T) crydenai.QueryIntent { + t.Helper() + s.mu.Lock() + defer s.mu.Unlock() + if len(s.ran) == 0 { + t.Fatal("no query reached the store") + } + return s.ran[len(s.ran)-1] +} + +func (s *fakeStore) count() int { + s.mu.Lock() + defer s.mu.Unlock() + return len(s.ran) +} + +// fakeCloser stands in for the snapshot's connection pool, so a test can +// see a rebuild releasing the pair it replaced. +type fakeCloser struct { + mu sync.Mutex + closed int +} + +func (c *fakeCloser) Close() error { + c.mu.Lock() + defer c.mu.Unlock() + c.closed++ + return nil +} + +type fixture struct { + t *testing.T + service *Service + secrets *settings.Secrets + provider *fakeProvider + store *fakeStore + + // builds counts factory calls, which is how the caching tests see a + // rebuild. closers is parallel to it. + builds int + closers []*fakeCloser +} + +func newFixture(t *testing.T) *fixture { + t.Helper() + + secrets, err := settings.NewSecrets(settings.NewMemoryStore(), "askai-test-encryption-key") + if err != nil { + t.Fatalf("NewSecrets: %v", err) + } + + f := &fixture{t: t, secrets: secrets, provider: &fakeProvider{}, store: &fakeStore{}} + f.service = New(secrets) + f.service.providers = func(settings.LLMProviderConfig, settings.DatabaseProviderConfig) (crydenai.LLMProvider, crydenai.QueryableStore, io.Closer, error) { + closer := &fakeCloser{} + f.builds++ + f.closers = append(f.closers, closer) + return f.provider, f.store, closer, nil + } + return f +} + +// putLLMConfig and friends write through Secrets, so the tests exercise +// the same seal-and-open path production reads. +func (f *fixture) putLLMConfig(cfg settings.LLMProviderConfig) { + f.t.Helper() + raw, err := settings.MarshalLLMProvider(cfg) + if err != nil { + f.t.Fatalf("MarshalLLMProvider: %v", err) + } + if err := f.secrets.Put(context.Background(), settings.KeyLLMProvider, raw); err != nil { + f.t.Fatalf("secrets.Put: %v", err) + } +} + +func (f *fixture) putDBConfig(cfg settings.DatabaseProviderConfig) { + f.t.Helper() + raw, err := settings.MarshalDatabaseProvider(cfg) + if err != nil { + f.t.Fatalf("MarshalDatabaseProvider: %v", err) + } + if err := f.secrets.Put(context.Background(), settings.KeyDatabaseProvider, raw); err != nil { + f.t.Fatalf("secrets.Put: %v", err) + } +} + +func (f *fixture) putWidgetConfig(cfg settings.AskAIWidgetConfig) { + f.t.Helper() + raw, err := settings.MarshalAskAIWidget(cfg) + if err != nil { + f.t.Fatalf("MarshalAskAIWidget: %v", err) + } + if err := f.secrets.Put(context.Background(), settings.KeyAskAIWidget, raw); err != nil { + f.t.Fatalf("secrets.Put: %v", err) + } +} + +func testLLMConfig() settings.LLMProviderConfig { + return settings.LLMProviderConfig{ + Kind: settings.LLMProviderKindAnthropic, + Model: "claude-opus-5", + APIKey: "sk-ant-a-test-credential", + MaxTokens: settings.DefaultLLMMaxTokens, + } +} + +func testDBConfig() settings.DatabaseProviderConfig { + return settings.DatabaseProviderConfig{ + Label: "the reporting replica", + DSN: "postgres://widget:a-password@127.0.0.1:5432/readonly?sslmode=disable", + MaxRows: settings.DefaultDatabaseMaxRows, + } +} + +func testWidgetConfig() settings.AskAIWidgetConfig { + return settings.AskAIWidgetConfig{ + Enabled: true, + AllowedOrigins: []string{allowedOrigin}, + Entities: []string{"sessions"}, + Greeting: "Ask about your account", + } +} + +// configure writes a complete, working deployment: both credentials and +// an enabled widget scoped to sessions. +func (f *fixture) configure() { + f.t.Helper() + f.putLLMConfig(testLLMConfig()) + f.putDBConfig(testDBConfig()) + f.putWidgetConfig(testWidgetConfig()) +} + +func (f *fixture) ask(question string) (widget.Answer, error) { + f.t.Helper() + return f.service.Ask(context.Background(), Request{ + OwnerUserID: owner, + Question: question, + Origin: allowedOrigin, + }) +} + +// ---------- the identity boundary ---------- + +// TestAskScopesTheQueryToTheAuthenticatedOwner is the test this package +// exists for. The model names somebody else; the store must never see +// it. cryden's widget.Ask overwrites the identity filter rather than +// validating it, so what arrives at the store is the owner and nothing +// the model said about identity survives. +func TestAskScopesTheQueryToTheAuthenticatedOwner(t *testing.T) { + f := newFixture(t) + f.configure() + f.provider.intent = crydenai.QueryIntent{ + Entity: "sessions", + Filters: []crydenai.QueryFilter{{Field: "user_id", Operator: "=", Value: otherUser}}, + } + + if _, err := f.ask("show me the other person's sessions"); err != nil { + t.Fatalf("Ask: %v", err) + } + + got := f.store.last(t) + if got.Entity != "sessions" { + t.Errorf("entity = %q, want sessions", got.Entity) + } + for _, filter := range got.Filters { + if filter.Field != "user_id" { + continue + } + if filter.Value != owner { + t.Errorf("user_id filter = %q, want the authenticated owner %q", filter.Value, owner) + } + if filter.Value == otherUser { + t.Errorf("the model's identity filter reached the store unchanged") + } + } +} + +// TestAskKeepsNonIdentityFilters pins the other half of the same +// behaviour: scoping replaces the identity filter and leaves everything +// else the model produced alone, because no other field can cross the +// identity boundary once user_id is forced. +func TestAskKeepsNonIdentityFilters(t *testing.T) { + f := newFixture(t) + f.configure() + f.provider.intent = crydenai.QueryIntent{ + Entity: "sessions", + Filters: []crydenai.QueryFilter{ + {Field: "user_id", Operator: "=", Value: otherUser}, + {Field: "ip", Operator: "=", Value: "203.0.113.7"}, + }, + } + + if _, err := f.ask("sessions from 203.0.113.7"); err != nil { + t.Fatalf("Ask: %v", err) + } + + got := f.store.last(t) + var sawIP, sawOwner bool + for _, filter := range got.Filters { + switch filter.Field { + case "ip": + sawIP = filter.Value == "203.0.113.7" + case "user_id": + sawOwner = filter.Value == owner + } + } + if !sawIP { + t.Errorf("the ip filter was dropped, filters = %+v", got.Filters) + } + if !sawOwner { + t.Errorf("the owner filter is missing, filters = %+v", got.Filters) + } +} + +// TestAskScopesAUsersQueryToTheCallersOwnRow covers the entity where the +// row IS the user, so every filter the model produced is discarded +// rather than merged. +func TestAskScopesAUsersQueryToTheCallersOwnRow(t *testing.T) { + f := newFixture(t) + config := testWidgetConfig() + config.Entities = []string{"users"} + f.putLLMConfig(testLLMConfig()) + f.putDBConfig(testDBConfig()) + f.putWidgetConfig(config) + + f.provider.intent = crydenai.QueryIntent{ + Entity: "users", + Filters: []crydenai.QueryFilter{ + {Field: "email", Operator: "=", Value: "someone.else@example.com"}, + }, + } + + if _, err := f.ask("what is my email"); err != nil { + t.Fatalf("Ask: %v", err) + } + + got := f.store.last(t) + if len(got.Filters) != 1 { + t.Fatalf("filters = %+v, want exactly the owner filter", got.Filters) + } + if got.Filters[0].Field != "id" || got.Filters[0].Value != owner { + t.Errorf("filters = %+v, want id = %q", got.Filters, owner) + } +} + +// TestAskRequiresAnOwner checks the guard rather than the happy path. An +// empty owner is the one input that would make widget.Ask scope a query +// to nobody, and it is refused before any provider is built. +func TestAskRequiresAnOwner(t *testing.T) { + f := newFixture(t) + f.configure() + + _, err := f.service.Ask(context.Background(), Request{ + Question: "who am i", + Origin: allowedOrigin, + }) + if !errors.Is(err, widget.ErrMissingOwner) { + t.Fatalf("Ask with no owner = %v, want widget.ErrMissingOwner", err) + } + if f.builds != 0 { + t.Errorf("providers built %d times for a request that had no owner", f.builds) + } +} + +// ---------- the deployment's own scope ---------- + +// TestAskRefusesAnEntityOutsideTheConfiguredScope is ScopedProvider +// reaching through the service: cryden allowlists audit_events, but this +// deployment configured sessions only, so the question is refused before +// it runs. +func TestAskRefusesAnEntityOutsideTheConfiguredScope(t *testing.T) { + f := newFixture(t) + f.configure() // entities: sessions + f.provider.intent = crydenai.QueryIntent{Entity: "audit_events"} + + _, err := f.ask("what has been recorded against me") + if !errors.Is(err, aiprovider.ErrEntityOutOfScope) { + t.Fatalf("Ask = %v, want aiprovider.ErrEntityOutOfScope", err) + } + if f.store.count() != 0 { + t.Errorf("a query ran for an entity outside the configured scope") + } +} + +// TestAskAllowsAnEntityInsideTheConfiguredScope is the control for the +// test above: the same entity runs once the operator has allowed it. +func TestAskAllowsAnEntityInsideTheConfiguredScope(t *testing.T) { + f := newFixture(t) + config := testWidgetConfig() + config.Entities = []string{"audit_events"} + f.putLLMConfig(testLLMConfig()) + f.putDBConfig(testDBConfig()) + f.putWidgetConfig(config) + + f.provider.intent = crydenai.QueryIntent{Entity: "audit_events"} + + if _, err := f.ask("what has been recorded against me"); err != nil { + t.Fatalf("Ask: %v", err) + } + if got := f.store.last(t); got.Entity != "audit_events" { + t.Errorf("entity = %q, want audit_events", got.Entity) + } +} + +// ---------- switched off, and not configured ---------- + +func TestAskRefusesADisabledWidget(t *testing.T) { + f := newFixture(t) + f.putLLMConfig(testLLMConfig()) + f.putDBConfig(testDBConfig()) + config := testWidgetConfig() + config.Enabled = false + f.putWidgetConfig(config) + + if _, err := f.ask("anything"); !errors.Is(err, ErrWidgetDisabled) { + t.Fatalf("Ask = %v, want ErrWidgetDisabled", err) + } +} + +// TestAskTreatsAnUnstoredWidgetConfigAsDisabled covers the deployment +// that has never opened the settings screen. Nothing stored reads as the +// zero value, which is disabled — the same answer the admin GET gives. +func TestAskTreatsAnUnstoredWidgetConfigAsDisabled(t *testing.T) { + f := newFixture(t) + f.putLLMConfig(testLLMConfig()) + f.putDBConfig(testDBConfig()) + + if _, err := f.ask("anything"); !errors.Is(err, ErrWidgetDisabled) { + t.Fatalf("Ask = %v, want ErrWidgetDisabled", err) + } +} + +// TestAskRefusesWhenAProviderIsNotConfigured covers both halves: an +// enabled widget with no LLM provider, and one with no database. Either +// missing means there is nothing to answer with. +func TestAskRefusesWhenAProviderIsNotConfigured(t *testing.T) { + tests := []struct { + name string + store bool // whether the LLM provider is stored + db bool // whether the database is stored + }{ + {name: "no LLM provider", db: true}, + {name: "no database", store: true}, + {name: "neither"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + f := newFixture(t) + f.putWidgetConfig(testWidgetConfig()) + if tt.store { + f.putLLMConfig(testLLMConfig()) + } + if tt.db { + f.putDBConfig(testDBConfig()) + } + + if _, err := f.ask("anything"); !errors.Is(err, ErrNotConfigured) { + t.Fatalf("Ask = %v, want ErrNotConfigured", err) + } + if f.builds != 0 { + t.Errorf("providers built %d times for an unconfigured widget", f.builds) + } + }) + } +} + +// ---------- origins ---------- + +func TestCheckOrigin(t *testing.T) { + allowed := []string{allowedOrigin, "http://localhost:3000"} + + tests := []struct { + name string + origin string + want error + }{ + {name: "an allowed origin", origin: allowedOrigin}, + {name: "an allowed origin with a default port spelled out", origin: "https://console.example.com:443"}, + {name: "an allowed origin in different case", origin: "HTTPS://Console.Example.COM"}, + {name: "a second allowed origin", origin: "http://localhost:3000"}, + { + name: "no origin at all", + origin: "", + // A non-browser caller sends no Origin and is not made to + // pretend otherwise; see checkOrigin on why refusing this + // would be theatre. + want: nil, + }, + {name: "a different host", origin: "https://evil.example.com", want: ErrOriginNotAllowed}, + {name: "a subdomain of an allowed origin", origin: "https://console.example.com.evil.test", want: ErrOriginNotAllowed}, + {name: "a different scheme", origin: "http://console.example.com", want: ErrOriginNotAllowed}, + {name: "a different port", origin: "https://console.example.com:8443", want: ErrOriginNotAllowed}, + {name: "a path", origin: "https://console.example.com/widget", want: ErrOriginNotAllowed}, + {name: "the literal null origin", origin: "null", want: ErrOriginNotAllowed}, + {name: "a bare host with no scheme", origin: "console.example.com", want: ErrOriginNotAllowed}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := checkOrigin(allowed, tt.origin) + if tt.want == nil { + if err != nil { + t.Fatalf("checkOrigin(%q) = %v, want nil", tt.origin, err) + } + return + } + if !errors.Is(err, tt.want) { + t.Fatalf("checkOrigin(%q) = %v, want %v", tt.origin, err, tt.want) + } + }) + } +} + +// TestAskRefusesAnOriginThatIsNotAllowed checks the check is wired into +// the request path and happens before anything is built or spent. +func TestAskRefusesAnOriginThatIsNotAllowed(t *testing.T) { + f := newFixture(t) + f.configure() + + _, err := f.service.Ask(context.Background(), Request{ + OwnerUserID: owner, + Question: "who am i", + Origin: "https://evil.example.com", + }) + if !errors.Is(err, ErrOriginNotAllowed) { + t.Fatalf("Ask = %v, want ErrOriginNotAllowed", err) + } + if f.builds != 0 { + t.Errorf("providers built %d times for a refused origin", f.builds) + } +} + +// TestAskRefusesADisabledWidgetBeforeAnOrigin keeps the two refusals in +// the right order: a deployment that has switched the widget off should +// not answer "that origin is not allowed", which would be true and would +// describe a feature that is not running. +func TestAskRefusesADisabledWidgetBeforeAnOrigin(t *testing.T) { + f := newFixture(t) + f.putLLMConfig(testLLMConfig()) + f.putDBConfig(testDBConfig()) + config := testWidgetConfig() + config.Enabled = false + f.putWidgetConfig(config) + + _, err := f.service.Ask(context.Background(), Request{ + OwnerUserID: owner, + Question: "anything", + Origin: "https://evil.example.com", + }) + if !errors.Is(err, ErrWidgetDisabled) { + t.Fatalf("Ask = %v, want ErrWidgetDisabled rather than an origin refusal", err) + } +} + +// ---------- questions ---------- + +func TestAskRejectsAnEmptyOrOversizedQuestion(t *testing.T) { + tests := []struct { + name string + question string + }{ + {name: "empty", question: ""}, + {name: "whitespace only", question: " \t "}, + {name: "over the limit", question: strings.Repeat("a", maxQuestionLength+1)}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + f := newFixture(t) + f.configure() + + if _, err := f.ask(tt.question); !errors.Is(err, ErrInvalidQuestion) { + t.Fatalf("Ask = %v, want ErrInvalidQuestion", err) + } + // Refused before the model is called: an empty question that + // reached the provider would be a model call that buys + // nothing. + if f.builds != 0 { + t.Errorf("providers built %d times for a refused question", f.builds) + } + }) + } +} + +func TestAskAcceptsAQuestionAtTheLimit(t *testing.T) { + f := newFixture(t) + f.configure() + f.provider.intent = crydenai.QueryIntent{Entity: "sessions"} + + if _, err := f.ask(strings.Repeat("a", maxQuestionLength)); err != nil { + t.Fatalf("Ask at the limit: %v", err) + } +} + +// ---------- the cache ---------- + +// TestAskRebuildsOnlyWhenTheStoredSettingsChange is the whole caching +// rule. Reading the settings on every question is what makes a saved +// change take effect without a restart; comparing them is what stops +// that from being a rebuild per question. +func TestAskRebuildsOnlyWhenTheStoredSettingsChange(t *testing.T) { + f := newFixture(t) + f.configure() + f.provider.intent = crydenai.QueryIntent{Entity: "sessions"} + + if _, err := f.ask("one"); err != nil { + t.Fatalf("Ask: %v", err) + } + if _, err := f.ask("two"); err != nil { + t.Fatalf("Ask: %v", err) + } + if f.builds != 1 { + t.Fatalf("built %d times for two questions with unchanged settings, want 1", f.builds) + } + + // A changed credential has to rebuild: it is the provider. + llm := testLLMConfig() + llm.APIKey = "sk-ant-a-rotated-credential" + f.putLLMConfig(llm) + + if _, err := f.ask("three"); err != nil { + t.Fatalf("Ask: %v", err) + } + if f.builds != 2 { + t.Fatalf("built %d times after a credential change, want 2", f.builds) + } + + // So does a changed scope, which is the ScopedProvider's input even + // though neither credential moved. + config := testWidgetConfig() + config.Entities = []string{"sessions", "audit_events"} + f.putWidgetConfig(config) + + if _, err := f.ask("four"); err != nil { + t.Fatalf("Ask: %v", err) + } + if f.builds != 3 { + t.Fatalf("built %d times after a scope change, want 3", f.builds) + } + + // And a change that does not reach the built pair does not rebuild: + // the greeting is copy, not configuration the providers hold. + config.Greeting = "Ask me anything" + f.putWidgetConfig(config) + + if _, err := f.ask("five"); err != nil { + t.Fatalf("Ask: %v", err) + } + if f.builds != 3 { + t.Fatalf("built %d times after a copy-only change, want 3", f.builds) + } +} + +// TestAskClosesTheProvidersItReplaces checks a rebuild releases the pool +// it is replacing, which is the leak this cache could otherwise have. +func TestAskClosesTheProvidersItReplaces(t *testing.T) { + f := newFixture(t) + f.configure() + f.provider.intent = crydenai.QueryIntent{Entity: "sessions"} + + if _, err := f.ask("one"); err != nil { + t.Fatalf("Ask: %v", err) + } + + llm := testLLMConfig() + llm.APIKey = "sk-ant-a-rotated-credential" + f.putLLMConfig(llm) + + if _, err := f.ask("two"); err != nil { + t.Fatalf("Ask: %v", err) + } + if len(f.closers) != 2 { + t.Fatalf("closers = %d, want 2", len(f.closers)) + } + if got := f.closers[0].closed; got != 1 { + t.Errorf("the replaced pair was closed %d times, want 1", got) + } + if got := f.closers[1].closed; got != 0 { + t.Errorf("the current pair was closed %d times, want 0", got) + } +} + +// TestCloseReleasesTheCurrentPair covers the shutdown path. Nothing in +// this repo calls it yet — there is no graceful shutdown to hang it off +// — so it is tested here rather than left to be discovered broken. +func TestCloseReleasesTheCurrentPair(t *testing.T) { + f := newFixture(t) + f.configure() + f.provider.intent = crydenai.QueryIntent{Entity: "sessions"} + + if _, err := f.ask("one"); err != nil { + t.Fatalf("Ask: %v", err) + } + if err := f.service.Close(); err != nil { + t.Fatalf("Close: %v", err) + } + if got := f.closers[0].closed; got != 1 { + t.Errorf("the current pair was closed %d times, want 1", got) + } + // Idempotent: a shutdown path that runs twice must not panic. + if err := f.service.Close(); err != nil { + t.Fatalf("second Close: %v", err) + } +} + +// TestFingerprintDoesNotCarryTheCredential is a small property with a +// large blast radius: the fingerprint is held for the life of the +// process, and two of its three inputs are secrets. +func TestFingerprintDoesNotCarryTheCredential(t *testing.T) { + llm := testLLMConfig() + db := testDBConfig() + + llmRaw, err := settings.MarshalLLMProvider(llm) + if err != nil { + t.Fatalf("MarshalLLMProvider: %v", err) + } + dbRaw, err := settings.MarshalDatabaseProvider(db) + if err != nil { + t.Fatalf("MarshalDatabaseProvider: %v", err) + } + + got := fingerprint(llmRaw, dbRaw, []string{"sessions"}) + for _, secret := range []string{llm.APIKey, db.DSN, "a-password"} { + if strings.Contains(got, secret) { + t.Errorf("the fingerprint contains %q", secret) + } + } + + // And it still tells two different settings apart. + if again := fingerprint(llmRaw, dbRaw, []string{"sessions"}); again != got { + t.Errorf("the same settings produced two fingerprints") + } + if other := fingerprint(llmRaw, dbRaw, []string{"audit_events"}); other == got { + t.Errorf("a scope change produced the same fingerprint") + } +} diff --git a/docs/development/CURRENT-STATE.md b/docs/development/CURRENT-STATE.md index 346bf3e..0518931 100644 --- a/docs/development/CURRENT-STATE.md +++ b/docs/development/CURRENT-STATE.md @@ -22,11 +22,15 @@ the config tuning advisor; Stage 2 is the AI provider settings — the LLM provider, the read-only database and the ask-ai widget config. Tier 4 is also where this repo stopped being purely a wrapper: it now ships a live `ai.LLMProvider` over the Anthropic SDK and a live `ai.QueryableStore` -over a second database connection, neither of which is wired to a -consumer yet. Every admin endpoint here is read-only except the +over a second database connection. Those two had no consumer when Tier 4 +landed; the ask-ai widget's serving endpoint, carried forward from that +tier and built after Tier 5, is the consumer — see the section on it +below. Every admin endpoint here is read-only except the `/v1/admin/settings/*` saves, which are the human half of the pre-fill-never-auto-apply rule — nothing on that surface applies a -suggestion by itself. +suggestion by itself. The widget's serving endpoint is not an admin +endpoint at all, and is the one AI-assisted surface here that answers an +end user rather than an operator. Tier 1 also added the second-factor surface: TOTP enroll/confirm/ disable, passkey registration/list/delete, magic-link request/complete, @@ -436,13 +440,15 @@ Three things in this stage are worth reading before touching them: `aiprovider.NewAnthropic` is a real `ai.LLMProvider` over the official Anthropic Go SDK, and `aiprovider.NewPostgresSnapshot` a real -`ai.QueryableStore`. **Nothing wires either from the stored config yet**: -the only consumer would be a widget serving endpoint, which does not -exist, so that glue lands with its first caller rather than being -written blind. `allowed_origins` is stored and validated but nothing -consults it at request time for the same reason, and the widget GET -carries no embed snippet because the URL in one would name a route this -repo does not serve. +`ai.QueryableStore`. **Nothing wired either from the stored config when +this stage landed** — Stage 2 stored the settings and enforced the scope +but had no caller, so the glue landed with its first one. That caller is +the widget's serving endpoint, built after Tier 5; see its own section +below. An earlier version of this paragraph also said `allowed_origins` +was consulted at no request time and that the widget GET carried no +embed snippet because the URL in one would name a route this repo does +not serve — both of those stopped being true at the same moment, and the +reason the snippet is still not returned is a different one. What Stage 2 does **not** have evidence for, and `PROGRESS.md` says in full: `CheckReadOnly` has never run against a real Postgres (the tested @@ -553,9 +559,103 @@ verified by reasoning and by the in-memory double, which reproduces the foreign key rather than accepting any id, so that the tested branch is the one production runs. `-race` was not run this session. -**Still not built** (unchanged from Tier 4, not part of this tier): the -widget's own serving endpoint, so `allowed_origins` remains stored and -unenforced; nothing constructs an `ai.LLMProvider` or -`ai.QueryableStore` from the stored config; and there is still no -graceful shutdown. +**Still not built** (unchanged from Tier 4, not part of this tier): +graceful shutdown, and per-user rate limiting on anything that calls a +model. The widget's own serving endpoint was in this list when Tier 5 +landed and is not any more — see the next section. + +## The ask-ai widget's serving endpoint — carried forward from Tier 4 + +Built on `feat/ask-ai-widget-serving`, after Tier 5 rather than with it. +`NEXT.md`'s Tier 4 carry-forward note asked for this to be picked up +"before or alongside Tier 6/7"; this is that, and it is the piece Tier 4 +Stage 2 deliberately left to its first caller rather than writing blind. + +Two new files and two small edits. `askai/` owns the glue — turning the +settings table into a live `ai.LLMProvider`/`ai.QueryableStore` pair — +and `httpapi/widget_handlers.go` owns the route. `POST /v1/ask-ai` is +registered with **`RequireAuth`, not `RequireAdmin`**, which is the +single most important thing about it. + +- **It is not an admin endpoint, and that is the feature.** cryden's + `widget` package exists for "a host application's own end users", and + `widget.Ask` takes an `ownerUserID` that the host must supply from its + own authentication. The widget answers questions about the signed-in + user's own sessions and audit events. Putting it behind `RequireAdmin` + would have been the easy mistake — every other AI surface here is + admin-only — and it would have made the widget unusable for every + person it is for. This was checked against three independent sources + before the route was written: cryden's `widget/ask.go` package doc and + `ownerUserID` contract, `README.md`'s own account of the widget, and + `settings/widget.go`'s note that `"*"` is refused because the widget + answers questions about the signed-in user. `NEXT.md` Tier 4's heading + — "AI-assisted admin endpoints (all behind `RequireAdmin`)" — is true + of the *settings* routes and was already misleading as a description + of this one, which Tier 4 listed but never built. +- **The owner id comes from the verified token and from nowhere else.** + The request body has no field that can name a user; one sent anyway is + ignored rather than rejected, because there is nothing for it to + reach. Two things hold that independently: `widget.Ask` discards the + model's identity filter and substitutes the real one, and the id + itself is read from the request context by `RequireAuth` rather than + from anything in the request. `TestAskAIScopesToTheTokenNotTheBody` + pins it end to end by sending a body that names another user and + asserting the `user_id` filter that reached the query surface was the + caller's. +- **`Composer` is nil, deliberately.** cryden documents a nil Composer + as "a valid, strictly safer default" and falls back to `RenderResult`, + a deterministic plain-text table. Supplying one means a second model + call per question whose output nothing validates, to turn a table into + prose. That is a decision to make on purpose if it is ever wanted, not + one to fall into by omission. +- **`allowed_origins` is finally consulted, and it is defense in depth + rather than the boundary.** The Bearer token is the boundary and is + verified first. A request carrying *no* `Origin` is allowed, on + purpose: refusing it would break every non-browser client — the + console itself, a mobile app, a test — while stopping nobody, because + a caller who can forge an allowlisted origin can equally omit it. What + the check buys is a guard against a stray embed on a site nobody meant + to authorise. +- **A real bug the tests caught, worth recording.** The first + `canonicalOrigin` used `url.Parse(...).Hostname()`, which ignores the + path — so `https://console.example.com/widget` canonicalised to the + same key as the allowed origin and was accepted. Fixed by refusing a + path, query, fragment or userinfo outright and requiring the scheme be + `http`/`https`, which mirrors `settings.validateWidgetOrigin`. The two + sides of one rule have to agree; a comparison that silently ignored a + path would accept a value the operator could never have stored. + Default ports are normalised away on both sides, because browsers omit + `:443` and an operator who typed it would otherwise have configured an + entry that could never match. +- **Settings are read per question; the built provider pair is cached on + a content fingerprint.** Three short reads and two AES-GCM opens + against one model call is a lopsided trade, and the failure mode of + the other side is a saved change that does not take effect until a + restart. The fingerprint is a **SHA-256 digest**, not the settings + themselves, because two of its three inputs are credentials and a + cache key lives as long as the process does. Rebuilds are cheap by + design — `NewPostgresSnapshot` uses `sql.Open`, which does not connect + — so a stored connection that has gone bad is reported by the query + that uses it rather than at startup. +- **The `Providers` seam is exported on purpose.** `askai.New` builds + the real Anthropic provider and Postgres snapshot; `NewWithProviders` + takes a factory. It exists because the security properties worth + testing — owner scoping, entity scoping, rebuild-on-change — cannot be + observed without a database otherwise, and the httpapi tests use it to + see the intent that actually reached the query surface. It is also the + hook a host running a different LLM backend needs, which is why it is + exported rather than a test-only accessor. +- **`Service.Close()` exists and nothing calls it.** There is still no + graceful shutdown for it to hang off, so it is there so that adding + one does not have to start by widening this type's API. + +**What is still owed, said plainly.** No per-user rate limiting on this +route: it spends money per question and is bounded only by the global +per-IP edge limiter. That needs policy — per-user or per-deployment, and +what number — which is a deployment's call rather than something to +invent here. `Service.Close()` is never called, for the shutdown reason +above. The Anthropic provider still has never called Anthropic, so the +live path from a question to a real model is exercised only through the +`Providers` seam with doubles; the wire shape is covered by +`aiprovider`'s own tests against a local fake. diff --git a/docs/development/NEXT.md b/docs/development/NEXT.md index c1a03fb..84c02ce 100644 --- a/docs/development/NEXT.md +++ b/docs/development/NEXT.md @@ -16,10 +16,16 @@ Tier 2 is done — see the status note under Tier 2 and `PROGRESS.md`'s migrations to copy. Tier 3 is done, in two stages on `feat/tier3-config-and-endpoints` — see the status note under Tier 3 and `PROGRESS.md`'s 2026-09-15 entries. -Tier 4 is **in progress** on `feat/tier4-ai-admin-endpoints`: Stage 1 -(digest + scheduling + history, support diagnosis, config tuning -advisor) is built — see the status note under Tier 4. Stage 2 (the LLM -and database providers, and the ask-AI widget config) is not started. +Tier 4 is done, both stages, on `feat/tier4-ai-admin-endpoints` — see +the status note under Tier 4. One thing from Tier 4 was carried forward +rather than left implicit — the ask-ai widget had configuration but no +serving endpoint — and **that is now done**, on +`feat/ask-ai-widget-serving`. It was picked up before Tier 6 rather +than buried under two new tiers, as the note below asked. See +`CURRENT-STATE.md`'s section on it; the short version is that the route +is `RequireAuth` rather than `RequireAdmin`, and the status note under +Tier 4 below carries a correction about why. +Tier 5 is done — see the status note under Tier 5 and `PROGRESS.md`. --- @@ -321,6 +327,23 @@ Two details were decided rather than assumed, and are recorded in > snippet, because the URL in one would name a route this repo does > not serve. > +> **Superseded.** The serving endpoint was built after Tier 5 on +> `feat/ask-ai-widget-serving`: `POST /v1/ask-ai` calls `widget.Ask`, +> `allowed_origins` is consulted on every question, and the pair the +> widget needs is constructed from the stored settings. Only the last +> sentence survives, and only its conclusion — the GET still returns +> no embed snippet, but no longer because the URL would 404. The +> reason now is that the markup belongs to the console. See +> `CURRENT-STATE.md`. +> +> **A correction to this tier's own heading, worth carrying.** Tier 4 +> is titled "AI-assisted admin endpoints (all behind `RequireAdmin`)", +> and that was never true of this item. The widget belongs to the +> signed-in end user and answers questions about their own account, so +> its route is `RequireAuth`. The heading is true of the *settings* +> routes; reading it as covering everything this tier listed is the +> mistake the carry-forward note above invited. +> > What is still owed from Stage 1: > > - the digest schedule is a goroutine on `context.Background()`, because @@ -531,3 +554,119 @@ tools' suggestions pre-fill. endpoint. The Stage 2 widget *configuration* exists, but nothing serves an embeddable widget, so `allowed_origins` is still stored and unenforced — the same gap `CURRENT-STATE.md` records for Tier 4. + +> **Closed.** That endpoint was built after Tier 5 on +> `feat/ask-ai-widget-serving`; `allowed_origins` is enforced at request +> time. What is still owed on it is per-user rate limiting, recorded in +> `CURRENT-STATE.md` and `PROGRESS.md` rather than guessed at here. + +--- + +## Tier 6 — SQLite backend, core auth only + +Scope decided in advance, don't relitigate: **core auth only.** Every +table this repo added on top of cryden for the admin console +(`operators`, `user_metadata`, `webhook_deliveries`, +`shipped_log_events`, `digest_runs`, `settings`, +`reviewed_anomalies`) stays Postgres-only. Whoever reaches for SQLite +is running something small and is extremely unlikely to also be +running the AI-assisted admin console — building that whole extra +table set twice for a case that probably won't use it is real ongoing +maintenance for close to zero benefit. If that changes later, it's its +own deliberate tier, not a quiet scope-creep of this one. + +- **Config**: a driver switch in `config/config.go` — `DATABASE_URL` + (Postgres, as today) or `SQLITE_PATH`, mutually exclusive, refuse to + start with both or neither set. `main.go` picks + `store/postgres.New*Store` or `store/sqlite.New*Store` accordingly + for cryden's own stores (`Users`, `Sessions`, `Audit`, + `Verifications`, `OAuth`, `TOTP`, `WebAuthn`, `RecoveryCodes`, + `APIKeys`, `Anomalies`) — every one of these already has a + `store/sqlite` implementation in cryden v2.5.0, this is wiring, not + new engine work. +- **Migrations**: don't renumber cryden's own SQLite migrations into + this repo's Postgres sequence (`001`-`014`) — they're a different + backend's schema, not the same history. Copy cryden's + `store/sqlite/migrations/*.sql` verbatim into a new + `migrations/sqlite/` directory, keeping cryden's own filenames, the + same "kept here so this repo is self-contained" reasoning as every + other copied migration. + + This is a straight 7-for-7 copy, not a consolidation: cryden's SQLite + migrations are `0001`-`0007`, the same count and the same filenames as + its Postgres ones. (An earlier version of this bullet claimed they were + "consolidated — fewer, larger files than the Postgres ones". They are + not, and the warning that followed from it — don't invent fake + incremental history — was written against a problem that does not + exist here. The instruction to keep cryden's filenames stands; the + reason is self-containment, not divergence.) If this repo ever needs + its own extra SQLite tables, those get their own numbered files + continuing this sequence rather than being folded into a cryden copy. +- **When `SQLITE_PATH` is set and an admin-console route is hit**: + decide and document this explicitly, don't leave it to whatever + happens to occur. The straightforward answer is a clean `501 + not_implemented_on_sqlite` for every route under `/v1/admin/*` that + touches a Postgres-only table (which is effectively all of them, + since `RequireAdmin` itself depends on `operators`) — so document + that the whole admin console, not just parts of it, requires + `DATABASE_URL`. Don't let it fail as a confusing 500 from a missing + table instead. +- **Verification**: run cryden's own `store/sqlite` test suite as + reference if unsure of a pragma or type mapping before writing + anything by hand — see cryden's `store/sqlite/SKILL.md`-equivalent + doc comments (`foreign_keys`, `busy_timeout`, `journal_mode(WAL)` are + load-bearing DSN pragmas, not optional). + +--- + +## Tier 7 — distribution: binaries, Docker, and migration DX + +The goal: `git clone` (or `docker run`), copy the env file, run, no +separate migrate step, no Go toolchain required for someone who isn't +a Go developer at all. + +**Worth knowing before starting: there is no migration runner in this +repo today, in any form.** `cmd/` holds only `grant-operator`, there is +no `embed.FS` anywhere in a non-test Go file, and CI applies migrations +by piping them through `psql`. So the first two bullets below are +greenfield rather than an extension of something existing, and the +"one implementation, called two ways" shape the `migrate` subcommand +bullet asks for is simply the design — there is no earlier runner to +avoid duplicating. + +- **Embed migrations into the binary** with `embed.FS` — both + `migrations/*.sql` (Postgres) and `migrations/sqlite/*.sql` from + Tier 6, so the running binary never depends on the source tree being + present next to it. +- **Auto-migrate on startup, on by default.** Before opening the + listening port, connect to the configured database, apply any + migration that hasn't run yet, in order, then start serving. +- **`SKIP_AUTO_MIGRATE=true`** as the escape hatch for teams who want a + controlled deploy step instead of migrations running silently on + every boot. When set, startup skips straight to serving. +- **A `migrate` subcommand** (`./api migrate`) that only applies + pending migrations and exits, no server started — this is what + `SKIP_AUTO_MIGRATE=true` deployments use as their explicit step. It + uses the same embedded files and the same apply function as the + automatic path: one runner written once, called from two places, + rather than two implementations of "run migrations" to keep in sync. +- **Binary releases**: extend the existing `.github/workflows/release.yml` + (already triggers on `v*` tags) to cross-compile and attach binaries + for `linux/amd64`, `linux/arm64`, `darwin/amd64`, `darwin/arm64`, and + `windows/amd64` to the GitHub Release. +- **Docker image**: a `Dockerfile` (multi-stage: build in a Go image, + run from a minimal base), published to a registry on the same tag + trigger. `docker run --env-file .env -p 8080:8080 ` should be + the entire setup instructions. +- **Graceful shutdown, pulled forward from Tier 3/4's owed list**: + this is the tier where it stops being a nice-to-have. A distributed + binary or container is exactly what a real orchestrator (Kubernetes, + Fly, Railway, plain systemd) sends `SIGTERM` to on every deploy, and + right now `main.go` ends at `log.Fatal(ListenAndServe(...))` with the + webhook worker and digest scheduler both running on + `context.Background()` — nothing stops them cleanly. Wire a real + shutdown context, cancel it on `SIGTERM`/`SIGINT`, and give + in-flight requests and the background workers a bounded grace period + before exiting. Don't ship distribution before this; a container + that gets killed mid-migration or mid-webhook-delivery on every + rolling deploy is a worse experience than the one being fixed. diff --git a/docs/development/PROGRESS.md b/docs/development/PROGRESS.md index 86e1804..fb4f0c2 100644 --- a/docs/development/PROGRESS.md +++ b/docs/development/PROGRESS.md @@ -1148,3 +1148,134 @@ it. every endpoint on this tier keys on the event id. It is a test double for a gap in cryden's own double, kept here rather than patched into the engine. + +## 2026-09-17 — the ask-ai widget's serving endpoint + +Branch `feat/ask-ai-widget-serving`, cut from +`feat/tier5-users-admin-surface` rather than `main`, because the whole +tier stack 2–5 is still unmerged and this sits on top of Tier 4 Stage 2's +settings work. This is the item `NEXT.md`'s Tier 4 carry-forward note +asked to be picked up "before or alongside Tier 6/7", done before Tier 6 +so two new tiers do not bury it. + +`go build ./...`, `go vet ./...`, `go test ./...` and `gofmt -l` are all +clean on this branch — the full suite, not just the two packages touched, +at 41s for `httpapi` alone. What was **not** run is the same list every +entry since Tier 4 has carried: no Postgres or Docker in this sandbox +(`permission denied … unix:///var/run/docker.sock`), so migrations +`001`–`014` remain unapplied, and `-race` was not run. + +### The decision that shaped everything else + +**`POST /v1/ask-ai` is behind `RequireAuth`, not `RequireAdmin`.** Every +other AI-assisted surface in this repo is admin-only, so this looks like +an oversight and is the opposite of one. cryden's `widget` package doc +says it is for "a host application's own end users", `widget.Ask` takes +an `ownerUserID` the host must supply from its own authentication, and +the widget answers questions about the signed-in user's own sessions and +audit events. Behind `RequireAdmin` it would be unusable for every person +it exists for. + +Checked against three independent sources before writing the route — +cryden's `widget/ask.go` doc and `ownerUserID` contract, this repo's +`README.md`, and `settings/widget.go`'s note that `"*"` is refused +because the widget answers about the signed-in user — and flagged to the +user before building, because it contradicts the heading of the tier +that specced it. `NEXT.md` Tier 4 is titled "AI-assisted admin endpoints +(all behind `RequireAdmin`)"; that is true of the *settings* routes and +was never true of this one, which Tier 4 listed but did not build. A +correction now sits under that heading. + +### Decisions worth not re-litigating + +- **Providers are cached on a content fingerprint, settings are read per + question.** Three short reads and two AES-GCM opens against one model + call that takes hundreds of milliseconds at best. The alternative — + cache the providers behind an invalidation the settings handlers fire + — fails silently when a future writer forgets to fire it. The + fingerprint is SHA-256, not the settings, because two of its three + inputs are credentials. +- **`Composer` stays nil**, so `widget.Ask` falls back to `RenderResult`. + cryden's doc calls nil "a valid, strictly safer default"; supplying one + is a second model call per question whose output nothing validates. +- **An absent `Origin` is allowed.** Documented at length on `checkOrigin` + because it reads like a hole. It is not: `Origin` is forgeable by + anything that is not a browser, so refusing an absent one would break + every non-browser client while stopping nobody — whoever can forge an + allowlisted origin can also omit it. The Bearer token is the boundary. +- **The `Providers` factory seam is exported**, not a test-only accessor. + Without it none of the properties worth testing — owner scoping, entity + scoping, rebuild-on-change — are observable without a live database. It + is also the hook a host on a different LLM backend needs, which is the + honest reason it is public rather than a test escape hatch. + +### A real bug the tests caught + +The first `canonicalOrigin` used `url.Parse(origin).Hostname()`, which +ignores the path — so `https://console.example.com/widget` canonicalised +to the same key as the allowed origin and was **accepted**. Caught by +`TestCheckOrigin`'s own table case for a path. + +Fixed by refusing a path, query, fragment or userinfo outright and +requiring the scheme be `http`/`https`, mirroring +`settings.validateWidgetOrigin`. The mirroring is the point: the +allowlist was stored under that rule, so a comparison that ignored a path +would accept a value the operator could never have stored, and the two +sides of one rule disagreeing is how a check stops meaning what its +comment says. Default ports are normalised away on both sides, because +browsers omit `:443` and an operator who typed it would otherwise have +configured an entry that silently never matched. + +### Stale premises corrected — four copies of one false claim + +Tier 4 Stage 2's widget GET documented that it returns no embed snippet +"because the URL in it would name an endpoint this repo does not serve +yet". That endpoint now exists, so the reason expired. The claim was +written in four places, and all four were corrected rather than left: +`httpapi/settings_handlers.go`'s doc comment, `README.md`, +`docs/development/CURRENT-STATE.md`, and `openapi/spec.yaml`. + +The conclusion survives — no snippet is still returned — but for a +different reason: the snippet is markup for the console's own pages, and +this repo has no opinion on another repo's markup. The endpoint path is +in the spec with every other path rather than returned as a string from a +settings GET. + +Also corrected under the same heading: `CURRENT-STATE.md`'s summary and +Stage 2 section both said nothing wires `ai.LLMProvider`/ +`ai.QueryableStore` from the stored config, which this work is. + +### Docs + +- **`openapi/spec.yaml` is now 1.7**, additive: the `POST /ask-ai` path + with its four error statuses, an `AskAIAnswer` schema, and a 1.7 + paragraph naming what makes this path different from its neighbours. + No existing path, field or status code changed. YAML re-parsed and + checked after editing (34 paths). +- `README.md` gained a "Serving the widget" section and the route line, + in the authenticated block where it belongs rather than the admin one. +- `CURRENT-STATE.md` gained its own section for this work and had the two + false claims above corrected in place. +- `NEXT.md`'s three carry-forward passages now record it as done. + +### Noticed while working, not fixed + +- **No per-user rate limiting on this route.** It spends deployment money + per question and is bounded only by the global per-IP + `EDGE_RATE_LIMIT`. A real limit needs policy — per-user or + per-deployment, and what number — which is a deployment's call rather + than something to invent here. Recorded in `README.md` and + `CURRENT-STATE.md` as owed. +- **`askai.Service.Close()` exists and nothing calls it**, for the same + reason `PROGRESS.md` has carried since Tier 3: this repo still has no + graceful shutdown. It is there so adding one does not have to start by + widening this type's API. +- **The live path to Anthropic is still unexercised.** The route is + tested through the `Providers` seam with doubles; `aiprovider`'s own + tests pin the wire shape against a local fake. Same gap Tier 4 Stage 2 + recorded, unchanged by this work. +- **`openapi/spec.yaml`'s route list is still incomplete** — the TOTP, + WebAuthn, magic-link, recovery-code and OAuth paths from Tier 1 have no + entries, as the Tier 5 entry noted. This work added its own path and + left that gap as it found it, since filling it is a Tier 1 + documentation pass. diff --git a/httpapi/errors.go b/httpapi/errors.go index f17c81d..e843a08 100644 --- a/httpapi/errors.go +++ b/httpapi/errors.go @@ -9,9 +9,11 @@ import ( "github.com/crydensync/cryden/v2/auth" "github.com/crydensync/cryden/v2/store" "github.com/crydensync/cryden/v2/token" + "github.com/crydensync/cryden/v2/widget" "github.com/crydensync/api/aiprovider" "github.com/crydensync/api/anomalyreview" + "github.com/crydensync/api/askai" "github.com/crydensync/api/settings" "github.com/crydensync/api/usermeta" ) @@ -54,6 +56,12 @@ var errEdgeRateLimited = errors.New("too many requests") // API rather than a 500 an operator would read as a bug. var errAdminStoresUnavailable = errors.New("this report requires stores that are not configured on this deployment") +// errAskAIUnavailable is the same wiring fact as errAdminStoresUnavailable +// one surface over: a router built without an askai.Service cannot serve +// the widget. 404 for the same reason — nothing is wrong with the server, +// the feature simply is not configured here. +var errAskAIUnavailable = errors.New("the ask-ai widget is not configured on this deployment") + // The following three are local, API-layer-only errors from the // OAuth redirect/callback flow itself — never returned by the engine, // which never touches HTTP or a specific provider. @@ -187,6 +195,33 @@ func mapError(err error) apiError { return apiError{http.StatusBadRequest, "database_role_not_read_only", "that database role can write, so it cannot back the AI query surface — create a role with SELECT only and use that"} case errors.Is(err, aiprovider.ErrCannotVerifyReadOnly): return apiError{http.StatusBadRequest, "database_role_unverified", "could not verify that the database role is read-only, so it was not stored — check the connection string and that the role can connect"} + // The ask-ai widget's serving side. Three of these are the same + // "not enabled here" family as the settings endpoints above: a + // deployment that has switched the widget off, or switched it on + // without configuring a provider, answers 404 so a console hides the + // launcher rather than reporting a fault. + // + // The entity refusal is one case covering two sentinels from two + // packages, and that is deliberate. aiprovider.ErrEntityOutOfScope is + // the deployment's own scope setting refusing, and + // widget.ErrEntityNotAvailable is cryden's fail-closed default for an + // entity it has not been taught to bound to one user. A caller can + // distinguish neither, and must not: the message names no entity and + // no scope, because it reaches an end user and describing this + // deployment's schema to whoever is typing questions at it is exactly + // what aiprovider.ScopedProvider exists to avoid. + case errors.Is(err, errAskAIUnavailable): + return apiError{http.StatusNotFound, "not_configured", "the ask-ai widget is not available on this deployment"} + case errors.Is(err, askai.ErrWidgetDisabled): + return apiError{http.StatusNotFound, "ask_ai_widget_disabled", "the ask-ai widget is switched off on this deployment"} + case errors.Is(err, askai.ErrNotConfigured): + return apiError{http.StatusNotFound, "not_configured", "the ask-ai widget is not available on this deployment"} + case errors.Is(err, askai.ErrOriginNotAllowed): + return apiError{http.StatusForbidden, "origin_not_allowed", "this page is not allowed to embed the ask-ai widget"} + case errors.Is(err, askai.ErrInvalidQuestion): + return apiError{http.StatusBadRequest, "invalid_question", "that question is empty or too long — see the message for which"} + case errors.Is(err, aiprovider.ErrEntityOutOfScope), errors.Is(err, widget.ErrEntityNotAvailable): + return apiError{http.StatusBadRequest, "question_not_answerable", "the widget cannot answer that kind of question on this deployment"} // A stored credential this deployment's key cannot open. Distinct // from "not configured" on purpose: the row is still there, and // telling an operator it is missing would send them to re-enter a diff --git a/httpapi/router.go b/httpapi/router.go index 38e18c6..059d42b 100644 --- a/httpapi/router.go +++ b/httpapi/router.go @@ -8,6 +8,7 @@ import ( "github.com/crydensync/cryden/v2/store" "github.com/crydensync/api/anomalyreview" + "github.com/crydensync/api/askai" "github.com/crydensync/api/config" "github.com/crydensync/api/digest" "github.com/crydensync/api/settings" @@ -92,6 +93,13 @@ type Deps struct { // the judgement lives here rather than in the engine's audit history. // See anomalyreview's package doc. Reviews anomalyreview.Store + + // AskAI backs the ask-ai widget's serving endpoint. Unlike every other + // AI-assisted dependency here it is not admin-scoped: it answers the + // signed-in end user's questions about their own account. Nil unless + // main.go built one; the handler then answers 404 rather than + // panicking. + AskAI *askai.Service } // NewRouter builds the full route table. Called once from main.go. @@ -120,6 +128,7 @@ func NewRouter(d Deps) http.Handler { tuning := &TuningHandlers{Audit: d.Audit, Config: d.Config} aiSettings := &SettingsHandlers{Secrets: d.Settings} anomalies := &AnomalyHandlers{Audit: d.Audit, Reviews: d.Reviews} + askAI := &WidgetHandlers{Service: d.AskAI} mux := http.NewServeMux() @@ -208,6 +217,15 @@ func NewRouter(d Deps) http.Handler { apiKeys.Revoke(w, r, r.PathValue("keyID")) })) + // The ask-ai widget. Authenticated as an ordinary end user rather than + // an operator, and that is the whole shape of the feature: it answers + // questions about the caller's OWN account, scoped by cryden's + // widget.Ask to the identity this repo verified from their token. It + // sits here among the authenticated routes rather than in the /v1/admin + // block below because an end user is not an operator and this is not an + // admin surface — see WidgetHandlers. + mux.HandleFunc("POST /v1/ask-ai", RequireAuth(engine, askAI.Ask)) + // Admin endpoints. Everything under /v1/admin goes through RequireAdmin // (middleware.go), which needs the `role` claim an operator's token // carries. OAuth provider health and the hash-migration report are both diff --git a/httpapi/settings_handlers.go b/httpapi/settings_handlers.go index caec50c..59728fc 100644 --- a/httpapi/settings_handlers.go +++ b/httpapi/settings_handlers.go @@ -290,12 +290,16 @@ func (h *SettingsHandlers) DeleteDatabaseProvider(w http.ResponseWriter, r *http // shows an operator anyway. See settings.AskAIWidgetConfig for why that // difference is real rather than an oversight. // -// There is also no embed snippet in the response, deliberately. The -// snippet is markup the csax+ console renders into its own pages, and the -// URL in it would name an endpoint this repo does not serve yet — so -// generating one here would be handing a console a script tag pointing at -// a 404. What this endpoint owes the console is the configuration the -// snippet is built from, which is exactly what it returns. +// There is also no embed snippet in the response. The snippet is markup +// the csax+ console renders into its own pages, and this package has no +// opinion on what another repo's pages should contain. An earlier +// version of this comment gave a different reason — that the URL in a +// snippet would name an endpoint this repo did not serve yet — and that +// one has expired: POST /v1/ask-ai serves the widget as of spec 1.7. +// What this endpoint owes the console is the configuration the snippet +// is built from, which is exactly what it returns; the path it posts to +// is in the spec with every other path, rather than returned as a string +// from here. func (h *SettingsHandlers) AskAIWidget(w http.ResponseWriter, r *http.Request) { if !h.Secrets.Configured() { writeErr(w, errSettingsNotConfigured) diff --git a/httpapi/widget_handlers.go b/httpapi/widget_handlers.go new file mode 100644 index 0000000..9234137 --- /dev/null +++ b/httpapi/widget_handlers.go @@ -0,0 +1,87 @@ +package httpapi + +import ( + "net/http" + + "github.com/crydensync/api/askai" +) + +// WidgetHandlers serves the ask-ai widget to the signed-in end user. +// +// This is the only AI-assisted surface in this repo that is not behind +// RequireAdmin, and the distinction is the point of the feature rather +// than an exception to it. Every other AI tool here answers an operator's +// question about the deployment; this one answers an ordinary user's +// question about their own account, and cryden's widget.Ask is built for +// exactly that caller — it takes the identity to scope to, and its +// package doc requires that identity to come from the host's own +// authentication of the current request. +// +// So the owner is UserIDFromContext, from the verified Bearer token, and +// the request body carries nothing but the question. It has no user id +// field to get wrong: a caller cannot ask about somebody else because +// there is nowhere to say who. Putting this behind RequireAdmin would +// make it unusable (end users are not operators) and would hand a token +// that can already read anyone's record a surface whose entire design +// assumes it reads exactly one person's. +// +// Read-only, in the sense CLAUDE.md means. widget.Ask parses, scopes and +// executes a SELECT through ai.QueryableStore, which has no method that +// can write; the connection it runs on is a role this repo verified +// refuses writes before it stored it. Nothing here can change an account. +type WidgetHandlers struct { + // Service is this repo's own serving side — see askai. Nil unless a + // router was built without one (tests); the handler then answers 404 + // rather than panicking on a nil dereference. + Service *askai.Service +} + +// askAIRequest is the body. One field, deliberately: see WidgetHandlers +// on why there is no owner here. +type askAIRequest struct { + Question string `json:"question"` +} + +// askAIResponse is the answer as a widget renders it. +// +// Text is cryden's RenderResult — a plain-text table — because no +// Composer is configured; see askai.Ask. It is a string and not a +// structured table so that adding a Composer later cannot change this +// response's shape: a model-written answer and a rendered one are both +// text, and a client that renders one renders the other. +type askAIResponse struct { + Answer string `json:"answer"` + // RowCount is how many rows the scoped query returned, which a + // widget shows as "3 results" when the table itself is collapsed. + RowCount int `json:"row_count"` +} + +// Ask — auth required, end user. Answers a question about the calling +// user's own account. +func (h *WidgetHandlers) Ask(w http.ResponseWriter, r *http.Request) { + if h.Service == nil { + writeErr(w, errAskAIUnavailable) + return + } + + var req askAIRequest + if err := decodeJSON(r, &req); err != nil { + writeBadRequest(w, "invalid request body") + return + } + + answer, err := h.Service.Ask(r.Context(), askai.Request{ + OwnerUserID: UserIDFromContext(r), + Question: req.Question, + Origin: r.Header.Get("Origin"), + }) + if err != nil { + writeErr(w, err) + return + } + + writeData(w, http.StatusOK, askAIResponse{ + Answer: answer.Text, + RowCount: len(answer.Result.Rows), + }) +} diff --git a/httpapi/widget_handlers_test.go b/httpapi/widget_handlers_test.go new file mode 100644 index 0000000..b43dd4b --- /dev/null +++ b/httpapi/widget_handlers_test.go @@ -0,0 +1,431 @@ +package httpapi + +import ( + "context" + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "strings" + "sync" + "testing" + + "github.com/crydensync/cryden/v2" + "github.com/crydensync/cryden/v2/store/memory" + "github.com/crydensync/cryden/v2/token" + + crydenai "github.com/crydensync/cryden/v2/ai" + + "github.com/crydensync/api/askai" + "github.com/crydensync/api/settings" +) + +// The widget is the one AI-assisted surface here that is not behind +// RequireAdmin, so these tests are as much about who gets through the +// route as about what it answers. + +type widgetFixture struct { + router http.Handler + store *settings.MemoryStore + + // seen records the intents that reached the query surface, which is + // where the identity assertion is made. + seen *intentRecorder + + userToken string + userID string + adminToken string +} + +// intentRecorder is the QueryableStore double. cryden's widget.Ask runs +// the query through here, so its contents are what the deployment would +// actually have executed. +type intentRecorder struct { + mu sync.Mutex + intents []crydenai.QueryIntent +} + +func (r *intentRecorder) RunSafeQuery(_ context.Context, intent crydenai.QueryIntent) (crydenai.QueryResult, error) { + r.mu.Lock() + defer r.mu.Unlock() + r.intents = append(r.intents, intent) + return crydenai.QueryResult{Columns: []string{"id"}, Rows: [][]string{{"s-1"}, {"s-2"}}}, nil +} + +func (r *intentRecorder) all() []crydenai.QueryIntent { + r.mu.Lock() + defer r.mu.Unlock() + return append([]crydenai.QueryIntent(nil), r.intents...) +} + +// widgetAnswerProvider returns a fixed intent, standing in for the model. +type widgetAnswerProvider struct{ intent crydenai.QueryIntent } + +func (p widgetAnswerProvider) ParseQueryIntent(_ context.Context, _ string) (crydenai.QueryIntent, error) { + return p.intent, nil +} + +// newWidgetFixture builds an engine on in-memory stores, an optional +// widget configuration, and a router whose query surface is the +// recorder. intent is what the model "produces"; withService=false +// leaves Deps.AskAI nil, which is the router a test that does not care +// about this feature gets. +func newWidgetFixture(t *testing.T, intent crydenai.QueryIntent, withService bool, configure func(*settings.Secrets)) widgetFixture { + t.Helper() + ctx := context.Background() + + var adminID string + engine, err := cryden.New(cryden.Config{ + JWTSecret: "test-secret", + Users: memory.NewUserStore(), + Sessions: memory.NewSessionStore(), + Audit: memory.NewAuditStore(), + Verifications: memory.NewVerificationStore(), + EmailSender: stubMailSender{}, + MagicLinkSender: stubMailSender{}, + AccessTokenClaims: token.ClaimsFunc(func(_ context.Context, userID string) (map[string]any, error) { + if userID == adminID { + return map[string]any{"role": "admin"}, nil + } + return nil, nil + }), + }) + if err != nil { + t.Fatalf("cryden.New on the in-memory stores: %v", err) + } + + admin, err := cryden.SignUp(ctx, engine, "operator@example.com", testPassword, "203.0.113.1") + if err != nil { + t.Fatalf("signup (operator): %v", err) + } + adminID = admin.ID + adminTokens, err := cryden.Login(ctx, engine, "operator@example.com", testPassword, "203.0.113.1", chromeOnMacOS) + if err != nil { + t.Fatalf("login (operator): %v", err) + } + + dana, err := cryden.SignUp(ctx, engine, "dana@example.com", testPassword, "203.0.113.2") + if err != nil { + t.Fatalf("signup (user): %v", err) + } + userTokens, err := cryden.Login(ctx, engine, "dana@example.com", testPassword, "203.0.113.2", chromeOnMacOS) + if err != nil { + t.Fatalf("login (user): %v", err) + } + + store := settings.NewMemoryStore() + secrets, err := settings.NewSecrets(store, testSettingsKey) + if err != nil { + t.Fatalf("settings.NewSecrets: %v", err) + } + if configure != nil { + configure(secrets) + } + + deps := Deps{Engine: engine, Settings: secrets} + seen := &intentRecorder{} + if withService { + deps.AskAI = askai.NewWithProviders(secrets, func(settings.LLMProviderConfig, settings.DatabaseProviderConfig) (crydenai.LLMProvider, crydenai.QueryableStore, io.Closer, error) { + return widgetAnswerProvider{intent: intent}, seen, nil, nil + }) + } + + return widgetFixture{ + router: NewRouter(deps), + store: store, + seen: seen, + userToken: userTokens.AccessToken, + userID: dana.ID, + adminToken: adminTokens.AccessToken, + } +} + +// sessionsIntent is what the model returns in the tests that are not +// about the scope refusal. +func sessionsIntent() crydenai.QueryIntent { + return crydenai.QueryIntent{Entity: "sessions"} +} + +const askAIPath = "/v1/ask-ai" + +const widgetTestOrigin = "https://console.example.com" + +// enableWidget writes a complete, working widget configuration directly +// through Secrets — the same sealed path the settings handlers use. +func enableWidget(t *testing.T, secrets *settings.Secrets, mutate func(*settings.AskAIWidgetConfig)) { + t.Helper() + ctx := context.Background() + + llmRaw, err := settings.MarshalLLMProvider(settings.LLMProviderConfig{ + Kind: settings.LLMProviderKindAnthropic, + Model: "claude-opus-5", + APIKey: "sk-ant-a-test-credential", + MaxTokens: settings.DefaultLLMMaxTokens, + }) + if err != nil { + t.Fatalf("MarshalLLMProvider: %v", err) + } + if err := secrets.Put(ctx, settings.KeyLLMProvider, llmRaw); err != nil { + t.Fatalf("secrets.Put (llm): %v", err) + } + + dbRaw, err := settings.MarshalDatabaseProvider(settings.DatabaseProviderConfig{ + Label: "the reporting replica", + DSN: "postgres://widget:a-password@127.0.0.1:5432/readonly?sslmode=disable", + MaxRows: settings.DefaultDatabaseMaxRows, + }) + if err != nil { + t.Fatalf("MarshalDatabaseProvider: %v", err) + } + if err := secrets.Put(ctx, settings.KeyDatabaseProvider, dbRaw); err != nil { + t.Fatalf("secrets.Put (db): %v", err) + } + + config := settings.AskAIWidgetConfig{ + Enabled: true, + AllowedOrigins: []string{widgetTestOrigin}, + Entities: []string{"sessions"}, + Greeting: "Ask about your account", + } + if mutate != nil { + mutate(&config) + } + widgetRaw, err := settings.MarshalAskAIWidget(config) + if err != nil { + t.Fatalf("MarshalAskAIWidget: %v", err) + } + if err := secrets.Put(ctx, settings.KeyAskAIWidget, widgetRaw); err != nil { + t.Fatalf("secrets.Put (widget): %v", err) + } +} + +func (f widgetFixture) do(t *testing.T, token, body string, headers map[string]string) *httptest.ResponseRecorder { + t.Helper() + req := httptest.NewRequest(http.MethodPost, askAIPath, strings.NewReader(body)) + if token != "" { + req.Header.Set("Authorization", "Bearer "+token) + } + for k, v := range headers { + req.Header.Set(k, v) + } + rec := httptest.NewRecorder() + f.router.ServeHTTP(rec, req) + return rec +} + +// ask posts a question as the end user from the allowed origin. +func (f widgetFixture) ask(t *testing.T, body string) *httptest.ResponseRecorder { + t.Helper() + return f.do(t, f.userToken, body, map[string]string{"Origin": widgetTestOrigin}) +} + +type askAIData struct { + Answer string `json:"answer"` + RowCount int `json:"row_count"` +} + +func decodeAskAI(t *testing.T, rec *httptest.ResponseRecorder) askAIData { + t.Helper() + var envelope struct { + Data askAIData `json:"data"` + } + if err := json.Unmarshal(rec.Body.Bytes(), &envelope); err != nil { + t.Fatalf("decoding %s: %v", rec.Body.String(), err) + } + return envelope.Data +} + +func decodeAPIError(t *testing.T, rec *httptest.ResponseRecorder) string { + t.Helper() + var envelope struct { + Error struct { + Code string `json:"code"` + } `json:"error"` + } + if err := json.Unmarshal(rec.Body.Bytes(), &envelope); err != nil { + t.Fatalf("decoding %s: %v", rec.Body.String(), err) + } + return envelope.Error.Code +} + +// TestAskAIRouteIsGatedByAuthNotAdmin is the shape of this feature in one +// test. The widget belongs to the signed-in end user, so a plain user +// token must get through — and an admin token must work too, because an +// operator is also a user of their own account. Failing this by requiring +// an operator would make the widget unusable for everyone it is for. +func TestAskAIRouteIsGatedByAuthNotAdmin(t *testing.T) { + f := newWidgetFixture(t, sessionsIntent(), true, func(secrets *settings.Secrets) { enableWidget(t, secrets, nil) }) + + if rec := f.do(t, "", `{"question":"when did i last log in"}`, nil); rec.Code != http.StatusUnauthorized { + t.Errorf("no token: status = %d, want 401", rec.Code) + } + if rec := f.do(t, "not-a-real-token", `{"question":"x"}`, nil); rec.Code != http.StatusUnauthorized { + t.Errorf("garbage token: status = %d, want 401", rec.Code) + } + if rec := f.ask(t, `{"question":"when did i last log in"}`); rec.Code != http.StatusOK { + t.Errorf("end user: status = %d, want 200 (body %s)", rec.Code, rec.Body.String()) + } + if rec := f.do(t, f.adminToken, `{"question":"when did i last log in"}`, map[string]string{"Origin": widgetTestOrigin}); rec.Code != http.StatusOK { + t.Errorf("operator asking about their own account: status = %d, want 200", rec.Code) + } +} + +// TestAskAIScopesToTheTokenNotTheBody is the security assertion at the +// HTTP layer. The body has no field to name a user, so the only identity +// available is the verified token — and this test pins that by sending a +// body that claims one anyway and checking the store never sees it. +func TestAskAIScopesToTheTokenNotTheBody(t *testing.T) { + f := newWidgetFixture(t, sessionsIntent(), true, func(secrets *settings.Secrets) { enableWidget(t, secrets, nil) }) + + rec := f.ask(t, `{"question":"show me everything","user_id":"99999999-9999-9999-9999-999999999999","owner_user_id":"99999999-9999-9999-9999-999999999999"}`) + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want 200 (body %s)", rec.Code, rec.Body.String()) + } + + intents := f.seen.all() + if len(intents) != 1 { + t.Fatalf("queries run = %d, want 1", len(intents)) + } + var sawOwner bool + for _, filter := range intents[0].Filters { + if filter.Field != "user_id" { + continue + } + sawOwner = true + if filter.Value != f.userID { + t.Errorf("user_id filter = %q, want the token's user %q", filter.Value, f.userID) + } + } + if !sawOwner { + t.Errorf("no user_id filter reached the store, filters = %+v", intents[0].Filters) + } +} + +// TestAskAIReturnsTheAnswerAndRowCount pins the response shape. +func TestAskAIReturnsTheAnswerAndRowCount(t *testing.T) { + f := newWidgetFixture(t, sessionsIntent(), true, func(secrets *settings.Secrets) { enableWidget(t, secrets, nil) }) + + rec := f.ask(t, `{"question":"when did i last log in"}`) + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want 200 (body %s)", rec.Code, rec.Body.String()) + } + + data := decodeAskAI(t, rec) + if !strings.Contains(data.Answer, "id") { + t.Errorf("answer = %q, want the rendered result table", data.Answer) + } + if data.RowCount != 2 { + t.Errorf("row_count = %d, want 2", data.RowCount) + } +} + +// TestAskAIRefusesAnOriginThatIsNotAllowed covers the embedding guard. +func TestAskAIRefusesAnOriginThatIsNotAllowed(t *testing.T) { + f := newWidgetFixture(t, sessionsIntent(), true, func(secrets *settings.Secrets) { enableWidget(t, secrets, nil) }) + + rec := f.do(t, f.userToken, `{"question":"x"}`, map[string]string{"Origin": "https://evil.example.com"}) + if rec.Code != http.StatusForbidden { + t.Fatalf("status = %d, want 403 (body %s)", rec.Code, rec.Body.String()) + } + if code := decodeAPIError(t, rec); code != "origin_not_allowed" { + t.Errorf("code = %q, want origin_not_allowed", code) + } + if len(f.seen.all()) != 0 { + t.Errorf("a query ran for a refused origin") + } +} + +// TestAskAIRefusesWhenTheWidgetIsOff covers both "switched off" and +// "never configured", which are one answer to a caller. +func TestAskAIRefusesWhenTheWidgetIsOff(t *testing.T) { + t.Run("disabled", func(t *testing.T) { + f := newWidgetFixture(t, sessionsIntent(), true, func(secrets *settings.Secrets) { + enableWidget(t, secrets, func(c *settings.AskAIWidgetConfig) { c.Enabled = false }) + }) + + rec := f.ask(t, `{"question":"x"}`) + if rec.Code != http.StatusNotFound { + t.Fatalf("status = %d, want 404 (body %s)", rec.Code, rec.Body.String()) + } + if code := decodeAPIError(t, rec); code != "ask_ai_widget_disabled" { + t.Errorf("code = %q, want ask_ai_widget_disabled", code) + } + }) + + t.Run("nothing stored", func(t *testing.T) { + // withService=true and no configure: the service exists and the + // settings store is empty, so the answer is "disabled" rather than + // the not_configured a nil service gives. + f := newWidgetFixture(t, sessionsIntent(), true, nil) + + rec := f.ask(t, `{"question":"x"}`) + if rec.Code != http.StatusNotFound { + t.Fatalf("status = %d, want 404 (body %s)", rec.Code, rec.Body.String()) + } + if code := decodeAPIError(t, rec); code != "ask_ai_widget_disabled" { + t.Errorf("code = %q, want ask_ai_widget_disabled", code) + } + }) +} + +// TestAskAIRefusesAnUnanswerableQuestion covers the scope refusal +// reaching the client. The deployment is configured for sessions only +// and the model names audit_events, so aiprovider.ScopedProvider refuses +// it — and the message must not say which entities exist, because it +// reaches whoever is typing questions at the widget. +func TestAskAIRefusesAnUnanswerableQuestion(t *testing.T) { + f := newWidgetFixture(t, crydenai.QueryIntent{Entity: "audit_events"}, true, func(secrets *settings.Secrets) { + enableWidget(t, secrets, nil) + }) + + rec := f.ask(t, `{"question":"what has been recorded against me"}`) + if rec.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want 400 (body %s)", rec.Code, rec.Body.String()) + } + if code := decodeAPIError(t, rec); code != "question_not_answerable" { + t.Errorf("code = %q, want question_not_answerable", code) + } + if body := rec.Body.String(); strings.Contains(body, "audit_events") || strings.Contains(body, "sessions") { + t.Errorf("the refusal names an entity: %s", body) + } + if len(f.seen.all()) != 0 { + t.Errorf("a query ran for an entity outside the configured scope") + } +} + +// TestAskAIRejectsAnEmptyQuestion covers the handler's own input check. +func TestAskAIRejectsAnEmptyQuestion(t *testing.T) { + f := newWidgetFixture(t, sessionsIntent(), true, func(secrets *settings.Secrets) { enableWidget(t, secrets, nil) }) + + for _, body := range []string{`{}`, `{"question":""}`, `{"question":" "}`} { + rec := f.ask(t, body) + if rec.Code != http.StatusBadRequest { + t.Errorf("%s: status = %d, want 400", body, rec.Code) + } + } +} + +func TestAskAIRejectsAMalformedBody(t *testing.T) { + f := newWidgetFixture(t, sessionsIntent(), true, func(secrets *settings.Secrets) { enableWidget(t, secrets, nil) }) + + rec := f.ask(t, `{"question":`) + if rec.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want 400", rec.Code) + } +} + +// TestAskAIWithoutAServiceRefusesRatherThanPanicking covers the router +// built with no askai.Service — what a test that does not care about +// this feature gets. The request is authenticated, so RequireAuth lets +// it through and the handler's own nil guard is what answers. +func TestAskAIWithoutAServiceRefusesRatherThanPanicking(t *testing.T) { + f := newWidgetFixture(t, sessionsIntent(), false, nil) + + rec := f.do(t, f.userToken, `{"question":"x"}`, map[string]string{"Origin": widgetTestOrigin}) + if rec.Code != http.StatusNotFound { + t.Fatalf("status = %d, want 404 (body %s)", rec.Code, rec.Body.String()) + } + if code := decodeAPIError(t, rec); code != "not_configured" { + t.Errorf("code = %q, want not_configured", code) + } +} diff --git a/main.go b/main.go index 9468dbd..18f5bc1 100644 --- a/main.go +++ b/main.go @@ -17,6 +17,7 @@ import ( "github.com/crydensync/cryden/v2/store/postgres" "github.com/crydensync/api/anomalyreview" + "github.com/crydensync/api/askai" "github.com/crydensync/api/config" "github.com/crydensync/api/digest" "github.com/crydensync/api/httpapi" @@ -390,6 +391,16 @@ func main() { Settings: settingsSecrets, Reviews: reviews, + + // Built over the same Secrets the settings endpoints write + // through, so a provider saved in the console is the one the + // widget's next question uses. Always constructed, even with no + // encryption key: it reads the settings on each question rather + // than being wired once at startup, so there is nothing to + // re-wire when an operator saves a change — it answers 404 + // not_configured until then, like every other unconfigured + // feature here. + AskAI: askai.New(settingsSecrets), }) limiter := httpapi.NewEdgeRateLimiter(cfg.EdgeRateLimit, cfg.EdgeRateLimitWindow) handler := httpapi.WithCORS(cfg.CORSOrigins, httpapi.WithEdgeRateLimit(limiter, router)) diff --git a/openapi/spec.yaml b/openapi/spec.yaml index c9bd1df..cb2cd02 100644 --- a/openapi/spec.yaml +++ b/openapi/spec.yaml @@ -1,7 +1,7 @@ openapi: 3.0.3 info: title: CrydenSync API - version: "1.6" + version: "1.7" description: > A self-hosted HTTP wrapper around the CrydenSync auth engine. Every response follows one of two envelope shapes: {"data": ...} @@ -85,6 +85,29 @@ info: side of the read-only rule. Nothing deletes a review: dismissing is a status, and withdrawing a judgement stores "unreviewed" rather than removing the row. + + 1.7 is additive, and adds the first path here that is neither an + authentication mechanic nor admin only: POST /ask-ai, the ask-ai + widget's serving endpoint. No existing path, field or status code + changed. + + It is gated on authentication rather than on the admin check, and + that is the shape of the feature rather than an oversight: the + widget answers questions about the caller's OWN account. The + identity is the verified token and nothing else — the request body + has no field that can name a user, and the engine's widget surface + discards whatever identity the model produced and substitutes the + one this API verified. 1.5's /admin/settings/ask-ai-widget + configures it and stays admin only; which entities it will answer + over is one of those settings, and a question naming anything + outside that set is refused before any query runs. + + Read-only, like every other AI-assisted surface here: it answers a + question and records nothing, and nothing in it can take an action + on the caller's account or anyone else's. It is also the first path + whose behaviour depends on a settings write taking effect without a + restart — it reads the stored provider configuration on every + question, so a change saved through 1.5 is in force on the next one. servers: - url: http://localhost:8080/v1 description: Local dev @@ -814,6 +837,32 @@ components: maxLength: 200 description: The input's placeholder text. Optional. + AskAIAnswer: + type: object + description: > + One answer from the widget, as POST /ask-ai returns it. + Deliberately just the text and a row count: the rows are already + in `answer`, rendered, and the count is what lets a client say + "3 sessions" without parsing the table back out of prose. + properties: + answer: + type: string + description: > + The answer as plain text — a rendered table of the result + rows, with the column names as a header line. Built + deterministically from the rows rather than composed by a + second model call, so the same question over the same data + yields the same string. No rows is the answer "No matching + results." rather than an empty string, because "nothing + matched" and "the widget failed to answer" must not look the + same to a client. + row_count: + type: integer + description: > + How many rows `answer` was rendered from, after the + deployment's row cap. Zero is a real answer and not an + error. + responses: BadRequest: description: Malformed request body @@ -1230,6 +1279,106 @@ paths: application/json: schema: { $ref: '#/components/schemas/ErrorResponse' } + /ask-ai: + post: + summary: Ask the widget a question about your own account + description: > + Authenticated as an ordinary end user, NOT as an operator. This + is the one AI-assisted surface on this API that is not behind + the admin check, and that difference is the feature: the answer + is about the caller's own sessions and audit events, not about + anyone else's. + + The identity is the verified token and nothing else. The request + body has no field that can name a user; one sent anyway is + ignored rather than rejected, because there is nothing for it to + reach. The engine's widget surface additionally discards whatever + identity filter the model produced and substitutes the one this + API verified, and it will not answer over an entity the + deployment has not enabled. + + Which entities this answers over is the `entities` setting on + /admin/settings/ask-ai-widget, which stays admin only. A + question naming anything outside the configured subset is + refused before a query runs. + + Read-only: it answers and records nothing, and nothing here can + take an action on any account. + + Answers are rendered from the query result rather than composed + by a second model call, so `answer` is a deterministic plain-text + table over rows already scoped to this one user. + + Two things worth knowing before calling it. It is slow by the + standards of the rest of this API — at minimum one call to the + configured model provider — and it costs the deployment money + per question, which is why `question` is length-bounded and why + this path is the one most worth putting a per-user limit in + front of. As of this version it is bounded only by whatever edge + rate limit the deployment runs. + security: [{ bearerAuth: [] }] + requestBody: + required: true + content: + application/json: + schema: + type: object + required: [question] + properties: + question: + type: string + maxLength: 500 + description: > + The question in whatever words the end user typed. + Empty or whitespace-only is refused. The 500 + character bound is on what the deployment will pay + to have parsed, not on what the model could read. + responses: + '200': + description: An answer scoped to the calling user. + content: + application/json: + schema: + type: object + properties: + data: { $ref: '#/components/schemas/AskAIAnswer' } + '400': + description: > + invalid_question — the question was empty, whitespace only, + or over 500 characters. question_not_answerable — the + question named an entity this deployment does not answer + over. Neither message names the entities that ARE available, + because this text reaches whoever is typing at the widget. + content: + application/json: + schema: { $ref: '#/components/schemas/ErrorResponse' } + '401': { $ref: '#/components/responses/Unauthorized' } + '403': + description: > + origin_not_allowed — the request carried an Origin header + naming an origin the operator has not allowlisted. This is a + guard against an embed on a site nobody meant to authorise, + NOT the security boundary; the Bearer token is. A request + with no Origin at all is allowed, deliberately: refusing it + would break every non-browser client while stopping nobody, + since a caller able to forge an allowlisted origin can also + simply omit it. + content: + application/json: + schema: { $ref: '#/components/schemas/ErrorResponse' } + '404': + description: > + ask_ai_widget_disabled — an operator has switched the widget + off. This is also the answer when nothing has been + configured, because the stored configuration's zero value is + disabled and an unconfigured widget is not a different thing + to a caller than a deliberately off one. not_configured — + the widget is on but the deployment has no LLM provider or + no read-only database stored. + content: + application/json: + schema: { $ref: '#/components/schemas/ErrorResponse' } + /admin/security/hash-migration: get: summary: Progress of a password-hash migration @@ -2384,11 +2533,19 @@ paths: operator anyway. There is deliberately no embed snippet in the response. The - snippet is markup the csax+ console renders into its own pages, - and the URL in it would name an endpoint this API does not serve - yet — so generating one here would hand a console a script tag - pointing at a 404. What this endpoint owes the console is the - configuration a snippet is built from, which is what it returns. + snippet is markup the csax+ console renders into its own pages — + the console's markup, the console's styling, the console's + script tag — and this API has no opinion on any of those. What + this endpoint owes the console is the configuration a snippet is + built from, which is what it returns. + + This API served no endpoint for a snippet to point at when that + was first written, and since 1.7 it does: /ask-ai serves the + widget. The reason for omitting a snippet is therefore no longer + that the URL in one would 404 — it is the ownership of the + markup. The path is documented here as its own path, which is + how a client learns every other path on this API, rather than + returned as a string from a settings GET. Nothing configured yet is a 200 with `enabled: false`, the zero value: a console then shows the feature as off rather than