diff --git a/.env.example b/.env.example index 1f4ec72..dfd4c01 100644 --- a/.env.example +++ b/.env.example @@ -35,6 +35,29 @@ WEBAUTHN_RP_ID= WEBAUTHN_RP_DISPLAY_NAME= WEBAUTHN_RP_ORIGINS= +# Seals the credentials this api stores ITSELF, in the settings table: +# the LLM provider's API key and the read-only database's password behind +# the AI-assisted admin features. AES-256-GCM, the same encryptor cryden +# uses for TOTP secrets, with the key derived from this value — so treat +# it with the same care as JWT_SECRET. +# +# Deliberately a separate value from ENCRYPTION_KEY rather than a reuse +# of it. That one is cryden's, the engine derives from it whatever it +# needs to read TOTP secrets, and the two have different lifetimes: a +# rotation of either must not silently make the other's rows unreadable. +# The same reasoning is why CLOUD_LOG_HASH_KEY is its own value. +# +# Leave it unset to run without the AI settings screens; GET/PUT +# /v1/admin/settings/llm-provider and its database counterpart then answer +# 404 not_configured rather than the server refusing to start, the same +# shape ENCRYPTION_KEY itself uses for the second factors. +# +# If it ever changes, rows written under the old value become unreadable +# and say so (a decryption failure, distinct from "never configured") — +# re-enter those credentials. Clearing a setting still works without the +# key, so there is a way out that is not direct database access. +SETTINGS_ENCRYPTION_KEY= + # Login anomaly detection and credential-stuffing detection. Both are # report-only — a flagged attempt writes an audit event and nothing else, # no login is ever blocked — and both are off until ANOMALY_DETECTION is @@ -67,6 +90,21 @@ REDIS_URL= RATE_LIMIT_ATTEMPTS= RATE_LIMIT_WINDOW_SECONDS= +# Account lockout: after LOCKOUT_THRESHOLD consecutive failed passwords the +# account is locked for LOCKOUT_DURATION_MINUTES. Defaults are cryden's own +# (5 and 15), restated here because the engine does NOT fill these in — it +# reads whatever it is handed, and both zero values are wrong in the same +# direction. A zero threshold locks every account on its first bad +# password; a zero duration locks it until an instant already past, which +# is to say never. LOCKOUT_THRESHOLD below 1 is refused rather than read as +# "off", because cryden has no way to switch lockout off. +# +# These are also what GET /v1/admin/config-tuning quotes when it says a +# lockout setting is in force, so the report and the engine cannot disagree +# about what this deployment is running. +LOCKOUT_THRESHOLD= +LOCKOUT_DURATION_MINUTES= + # Password hashing. bcrypt is the engine's default; argon2id is the # current recommendation for new deployments (memory-hard, and the knob a # GPU attacker cannot parallelize around). Switching is safe at any time @@ -154,3 +192,19 @@ WEBHOOK_URL= WEBHOOK_SECRET= WEBHOOK_EVENTS= WEBHOOK_MAX_ATTEMPTS= + +# The weekly digest schedule. Unset (or 0) means no schedule at all: no +# goroutine runs, nothing is written, and GET /v1/admin/digest/history +# answers 404 not_configured. GET /v1/admin/digest works either way — it +# builds a digest on demand and records nothing. +# +# Set it and this repo builds the engine's digest every N hours and stores +# the rendered report in the digest_runs table, which is what the history +# endpoint lists. 168 is weekly. cryden has no scheduling concept, so this +# table, this job and that endpoint are all this repo's own. +# +# The first run happens one full interval after startup, not at boot: a +# process that restarts more often than the interval elapses would +# otherwise write one row per restart, and a history that grows with +# restarts is not a history of anything. +DIGEST_INTERVAL_HOURS= diff --git a/CLAUDE.md b/CLAUDE.md index efe88a7..3519948 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1,4 +1,4 @@ -# CODEX.md — working conventions for this repo +# CLAUDE.md — working conventions for this repo Read this fully before writing any code. Then read `docs/development/CURRENT-STATE.md` (what exists and why) and `docs/development/NEXT.md` (the ordered queue, diff --git a/README.md b/README.md index 17de229..21a0cc4 100644 --- a/README.md +++ b/README.md @@ -235,6 +235,17 @@ Set `PASSWORD_HASHER=argon2id` and every login whose stored hash is out of date - `estimated_remaining` is therefore *estimated*, floored at zero, and is `total_users - upgraded_events`. - `upgraded_events_in_window` is the field that actually answers "is this draining": the all-time count only ever rises, while a windowed one falls to zero as the last stragglers log in. `window_days` (1–365, default 7) sets that window. +`GET /v1/admin/support/diagnose?email=` answers the support ticket "why can't this person log in", from the account's own recorded history: whether it is locked and until when, its consecutive failed-attempt count, how many sessions it currently holds, and the recent failure-type events behind all of that, newest first. + +```json +{"data": {"email": "dana@example.com", "text": "Login diagnosis for dana@example.com\n\nAccount is LOCKED until 14:32 UTC.\n5 consecutive failed attempts currently recorded…"}} +``` + +- **An unknown address is the answer, not a 404.** cryden's `admin.DiagnoseLogin` returns `Found: false` rather than an error, and the report says "No account exists for this email address." A 404 would be indistinguishable from a broken endpoint, and "you have the wrong address" is exactly what a support agent pasting a typo'd email needs to be told. +- The text is the engine's, passed through verbatim. This repo does not reformat a report it does not own — and `email` is echoed alongside it so an agent working through a queue can see which address was answered. +- **Read-only structurally, not by convention.** The report is built through interfaces carrying no `LockAccount`, `ResetFailedAttempts` or `Revoke`, so the endpoint cannot unlock the very account it is describing, whatever the caller asks for. That is cryden's design and this repo adds nothing on top of it. +- A missing `email` is a `400`, not a diagnosis of the empty string — which would come back as "no account exists", an answer to a question nobody asked. + ## API keys `POST /v1/api-keys` mints a machine-to-machine credential for the calling user and returns the raw key **once** — cryden stores only its SHA-256 hash and can never reproduce it, so a caller that loses it has to mint a new one. The response carries the raw key, the stored record (`id`, `name`, `prefix`, `scopes`, `expires_at`, `expired`, `created_at`, `last_used_at`) and a `notice` saying so; a client that renders the key without that notice is the failure this guards against. @@ -315,6 +326,92 @@ There is no vendor here: this repo ships no SDK, so "shipped" means "recorded in - An unknown `level` is a `400` naming the four valid values, not an empty list — which is indistinguishable from "the engine has been quiet". - The write is **synchronous**, on the goroutine that logged. That is a real cost and is not the shape a busy deployment wants; it is the shape this one can have, because an asynchronous sink needs a flush policy and a shutdown path, and this repo has no graceful shutdown anywhere yet. A buffer that is never flushed on exit is a log that silently drops its last records before a crash, which for a log is the failure that matters most. `LOG_LEVEL` (default `info`) is what keeps the volume sane in the meantime, since the engine's debug records never reach the sink. +## Config tuning advisor + +`GET /v1/admin/config-tuning?window_days=` reads the recent audit history, compares it against the settings **actually in force**, and returns the suggestions as a structured list — one object per knob, ready to render as a card: + +```json +{"data": { + "since": "…", "until": "…", "window_days": 30, + "counts": {"account_locked": 5, "login_failed": 5}, + "suggestions": [{ + "area": "Lockout", + "finding": "5 accounts were locked out of 5 recorded in this window (100%) — LockoutThreshold is currently 5, LockoutDuration 15m0s.", + "suggestion": "If most of these are real users mistyping a password rather than an attack, consider raising LockoutThreshold…" + }] +}} +``` + +- **There is no write path, and there is not going to be one.** No parameter changes a setting, and no counterpart endpoint applies a suggestion. The decision recorded for this surface is **pre-fill, never auto-apply**: a suggestion pre-fills the settings field it concerns, and a human still saves that change through the ordinary settings path. An endpoint that wrote a suggested value straight into live config would be the violation `CLAUDE.md`'s hard rule names, and would let a bad suggestion change production with no confirmation. The route accepts `GET` and nothing else. +- It calls `admin.BuildTuningReport` **directly**, not the flattened `cryden.ConfigTuningReport` text helper. The text is right for a CLI and wrong for a console: a pre-rendered blob cannot become one card per suggestion, and a client would be back to parsing English to find which knob a paragraph was about. +- `counts` is the raw audit evidence the suggestions were computed from, including event types cryden does not define — so a console can show the numbers rather than asking an operator to trust a sentence. +- Every value in the report comes from the config this process built the engine from, never a second reading of the environment. `UsingDefaultRateLimiter` is derived from `RedisURL` being empty — the same condition `main.go` uses to build the Redis limiter — so the report and the wiring cannot drift. +- `window_days` (1–365) defaults to cryden's own **30**-day tuning window, deliberately wider than the digest's week: a config knob should be judged against a month of traffic, not whatever happened this week. +- The password-strength finding always says the breach checker is not set. That is **accurate rather than a stub**: this repo has never wired `Config.BreachedPasswordChecker`, because cryden ships no implementation and every real one calls somebody else's corpus. + +`LOCKOUT_THRESHOLD` and `LOCKOUT_DURATION_MINUTES` (defaults 5 and 15 minutes) are passed through to the engine explicitly, for the two reasons above at once: cryden reads them straight off its config with no defaulting, so a deployment that left them implicit was running a lockout that could never actually trigger; and the tuning report describes the settings in force, so it should not have to guess what the engine was handed. + +## Weekly digest + +Two endpoints, and only one of them depends on any configuration: + +``` +GET /v1/admin/digest?window_days= # built now, records nothing +GET /v1/admin/digest/history?limit= # what the schedule recorded +``` + +`GET /v1/admin/digest` returns `cryden.DigestSince`'s report verbatim — the text is the engine's, and this repo does not reformat a report it does not own — alongside `since` and `until`, because a client should not have to parse English out of a digest to learn what it covers. `window_days` (1–365, default 7) is passed to the engine rather than implemented here; the seven-day default is what makes it a *weekly* digest. **Asking twice leaves no trace**: the endpoint records nothing, and if that ever stopped being true an operator could no longer tell what the schedule produced from what somebody happened to open. + +Setting `DIGEST_INTERVAL_HOURS` (168 is weekly) turns on the schedule: a background job calls the same report every N hours and writes the rendered result to the `digest_runs` table, which `GET /v1/admin/digest/history` lists newest first. Unset means no schedule — no goroutine runs, nothing is written, and the history endpoint answers `404 not_configured` rather than an empty list an operator would read as "nothing has ever happened". + +- **cryden has no scheduling concept.** `WeeklyDigest`/`DigestSince` build a report on demand and return a string; there is no run record and nothing that remembers a digest was ever generated. So the table, the job and the history endpoint are entirely this repo's own. +- **The row is the report, not a recipe for one.** The rendered text is stored rather than the counts behind it, because a digest covers a window that has *ended*: re-running its query later would not reproduce it, since "the last seven days" is anchored to when it was built. +- **The first run is one full interval after startup**, not at boot. A process that restarts more often than the interval elapses — a crashloop, a deploy pipeline, a laptop — would otherwise write one row per restart, and a history that grows with restarts rather than with time is not a history of anything. +- A failed run is **logged and swallowed**. This runs in a goroutine with nobody to hand an error to, and a scheduler that stopped at the first database blip would silently stop producing digests for the rest of the process's life. +- Nothing on the HTTP surface can create a digest run. Only the scheduler writes, and it is a process component rather than a request handler — the read-only rule the whole admin surface follows. + +## AI provider settings + +The AI-assisted admin features need two things only this repo can supply, because cryden defines them as interfaces the host implements: an `ai.LLMProvider` that turns a question into a `QueryIntent`, and an `ai.QueryableStore` that runs one. Three settings endpoints configure them, all operator-only: + +``` +GET|PUT|DELETE /v1/admin/settings/llm-provider +GET|PUT|DELETE /v1/admin/settings/database-provider +GET|PUT|DELETE /v1/admin/settings/ask-ai-widget +``` + +These are the admin surface's **only** writes, and they are the other half of the read-only rule rather than a hole in it. A tuning suggestion pre-fills one of these forms; an operator presses save; this is what handles that save. No AI-assisted handler in this repo holds a reference to any of them, and none accepts a suggestion as input. + +All three answer `404 not_configured` when `SETTINGS_ENCRYPTION_KEY` is unset — without a key there is nowhere safe to put a credential, so the API refuses rather than storing one in the clear. + +### Credentials + +The LLM API key and the database connection string are sealed with **AES-256-GCM before they reach the table**, keyed from `SETTINGS_ENCRYPTION_KEY`. Treat it like `JWT_SECRET`: set it, keep it out of source control, and expect a rotation to need the old value for as long as rows written under it exist. + +- **Separate from cryden's `ENCRYPTION_KEY` on purpose.** The two seal different things with different lifetimes — cryden's covers what the engine stores (TOTP secrets), this one covers what the API stores — so one leaking or rotating need not touch the other. Same reasoning as `CLOUD_LOG_HASH_KEY`. +- **Never returned, in any form.** Not masked, not truncated to the last four characters: `api_key_set` and `dsn_set` booleans are what a console renders "saved" from, and returning any part of the value would put it in a browser's memory and a devtools panel. `GET` on the database provider returns the host and database name only, so an operator can tell which connection is stored without being shown a password. +- **A changed key is `409 setting_undecryptable`, not `404`.** The row is still there, and reporting it as missing would send an operator to re-enter a credential that is fine. `DELETE` needs no key at all, which is what makes it the way out for a deployment that has lost one. +- **The credential is required on every `PUT`**, rather than optional with "blank means keep the existing one". That convention is the usual one and it is wrong here: an omitted field and a deliberately cleared one would be the same request, and getting it wrong means a form that appears to save a key while silently storing an empty one. + +### The read-only database requirement + +`PUT /v1/admin/settings/database-provider` **connects with the supplied credentials and attempts a write before storing anything.** cryden's own position is that for this feature "the credential boundary, not just the allowlist, is the real safety guarantee" — a role that cannot `INSERT` cannot `INSERT` whatever the query builder does with its input. So the order is: validate the shape, prove the role cannot write, and only then store. + +- **An attempted write, not a reading of the role's attributes.** A role with `rolsuper` set, or a connection string containing the word "readonly", or a "read-only?" checkbox in the console, are all claims. Only the server's refusal is evidence, and it is evidence about the actual role, on the actual database, through the actual credentials. It cannot be done client-side either — a browser cannot open a Postgres connection. +- **The probe writes to `pg_temp`**, the session's own temporary schema. The table lives only for the life of that connection and is dropped when it closes, so a probe that fails leaves nothing for an operator to clean up. The pool is capped at one connection so the `CREATE` and the `INSERT` share the session that owns the temp table — a pool that split them would have the `INSERT` fail on a missing table, which looks like a refusal and is not one. +- **Three outcomes, deliberately distinct.** A refused write is a pass. A successful write is `400 database_role_not_read_only`. Anything else — no connection, a timeout, a `CREATE` that failed for a reason other than privilege — is `400 database_role_unverified`, **which is not a pass**. Treating "could not find out" as success would make the check pass exactly when it is least able to tell. Only Postgres' own SQLSTATE `42501` (`insufficient_privilege`) counts as a refusal, and it is matched by code rather than by message, since the message is localized and reworded between major versions. +- **It is slower than its neighbours**, because it opens a connection and runs statements before answering, bounded by a ten-second timeout. That is paid once per save, not per query. + +### The ask-ai widget + +`GET|PUT /v1/admin/settings/ask-ai-widget` stores the widget's enabled flag, the origins allowed to embed it, the entities it answers over, and its copy. It is the one setting here that is **not** a credential, so there is nothing to redact. + +- **`entities` has teeth.** cryden's `widget.Ask` force-scopes every parsed intent to the calling end user's own rows — it discards whatever identity filter the model produced and substitutes the real one, rather than validating and rejecting, so there is no oracle — but it scopes over the whole of `ai.AllowedEntities`. Narrowing that further is a host decision, so `aiprovider.ScopedProvider` enforces the configured subset in front of the provider. A scope setting nothing consulted would be worse than no setting at all. The list is validated against cryden's own allowlist rather than a copy of it. +- **`"*"` 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. + ## Design notes - `CORS_ORIGINS` is required, no wildcard default — an API handling auth tokens should never allow every origin. @@ -324,8 +421,8 @@ There is no vendor here: this repo ships no SDK, so "shipped" means "recorded in - A paused login is a `200`, not an error: nothing failed, the caller just has one more step. `httpapi/second_factor.go` is the one place that response shape is written. - `DELETE /v1/passkeys/{credentialID}` takes a JSON body (`{"password": "..."}`) — the password is re-confirmation, so a stolen access token alone cannot weaken an account's own auth requirements. - Passkey ceremony options and the browser's credential response travel as raw JSON (an object, not a JSON-encoded string), since that is exactly what `navigator.credentials.create()`/`.get()` produce and consume. -- **Three of this repo's tables are not cryden's and never will be**: `user_metadata`, `webhook_deliveries`, `shipped_log_events`. cryden calls an interface and moves on; it keeps no queryable history of what a sender or a logger did, and `store.User` has no metadata concept on purpose. Each lives in its own package (`usermeta/`, `webhook/`, `shiplog/`) with a Postgres store and an in-memory double behind one interface, mirroring the `store/interfaces.go` + `store/memory` + `store/postgres` split cryden itself uses — which is what makes an endpoint over them testable with no database. -- **The admin surface is read-only by construction.** `GET /v1/admin/webhooks/deliveries` and `GET /v1/admin/logging/recent` report; neither offers a "retry this delivery" button, a "replay this event", or any way to write a log record or a delivery row. That is the same rule cryden's AI admin tools are built under, carried across the repo boundary: an operator reads the state of the system, and every change to it goes through the explicit path that owns that change (or through the receiving system, for a delivery). Adding a write here is a design change, not a convenience. +- **Five of this repo's tables are not cryden's and never will be**: `user_metadata`, `webhook_deliveries`, `shipped_log_events`, `digest_runs` and `settings`. cryden calls an interface and moves on; it keeps no queryable history of what a sender or a logger did, no schedule, no run record, and no configuration storage — and `store.User` has no metadata concept on purpose. Each lives in its own package (`usermeta/`, `webhook/`, `shiplog/`, `digest/`, `settings/`) with a Postgres store and an in-memory double behind one interface, mirroring the `store/interfaces.go` + `store/memory` + `store/postgres` split cryden itself uses — which is what makes an endpoint over them testable with no database. +- **The admin surface is read-only by construction, with one named exception.** `GET /v1/admin/webhooks/deliveries` and `GET /v1/admin/logging/recent` report; neither offers a "retry this delivery" button, a "replay this event", or any way to write a log record or a delivery row. That is the same rule cryden's AI admin tools are built under, carried across the repo boundary: an operator reads the state of the system, and every change to it goes through the explicit path that owns that change (or through the receiving system, for a delivery). Adding a write here is a design change, not a convenience. **The exception is `/v1/admin/settings/*`**, which is a settings save — the "a human still saves it" half of the pre-fill rule, not an action any AI tool can reach. Its credentials are encrypted at rest, and it is the only place in this API that stores one. If you are adding a write under `/v1/admin` that is not a settings save, the answer is no. - `webhook_deliveries.id` is a `BIGSERIAL` surrogate key rather than the natural key you might expect. The event id it corresponds to **can be empty** — cryden generates it with `crypto/rand` and deliberately delivers an event without one rather than dropping it — and a delivery log whose primary key could be blank is a log that loses exactly the rows you would most want to see. The engine's own id is recorded beside it as `event_id` and is used for the receiver's idempotency. - This repo has **no graceful shutdown**, and as of this tier that is a stated gap rather than an unnoticed one: `main.go` ends at `log.Fatal(http.ListenAndServe(...))`, so the webhook worker's context is never cancelled and the shipped-events sink has no flush-and-exit path. Both were built so that adding one later is a change to `main.go` alone — the worker takes a `context.Context`, which today is `context.Background()`. The sink writes synchronously for the same reason: a buffered sink with no shutdown path drops its last records on a crash. diff --git a/aiprovider/anthropic.go b/aiprovider/anthropic.go new file mode 100644 index 0000000..b66edfa --- /dev/null +++ b/aiprovider/anthropic.go @@ -0,0 +1,287 @@ +// Package aiprovider holds this repo's implementations of the interfaces +// cryden's ai and widget packages define. +// +// cryden ships none on purpose: ai.LLMProvider and ai.QueryableStore are +// shaped so a host brings its own vendor and its own database connection +// (see ai/types.go — "Ships zero implementations here — the consumer +// brings its own provider and API key, the same pattern as +// notify.EmailSender and logger.Logger"). This is that consumer. +// +// Nothing in this package is the safety boundary. cryden validates every +// QueryIntent against its allowlist before any query runs (ai.validateIntent) +// and widget.Ask force-scopes every intent to the calling user before +// that. What happens here is narrower and easier to state: turn a +// question into a candidate intent, and run an already-validated intent +// against a connection that has been checked to be read-only. +package aiprovider + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "strings" + + "github.com/anthropics/anthropic-sdk-go" + "github.com/anthropics/anthropic-sdk-go/option" + + crydenai "github.com/crydensync/cryden/v2/ai" +) + +// ErrNoAPIKey is returned when the provider was built without a +// credential. Constructing one anyway and failing at the first call would +// move the failure from startup to the middle of an admin's question. +var ErrNoAPIKey = errors.New("aiprovider: an API key is required") + +// ErrUnexpectedAnswer means the model returned something that is not a +// QueryIntent. It is a real possibility even with the output schema +// enforced — a refusal, a truncated response — and it is reported as its +// own error rather than as a validation failure, because "the model did +// not answer the question" and "the model answered with something unsafe" +// call for different words in front of an operator. +var ErrUnexpectedAnswer = errors.New("aiprovider: the model did not return a query intent") + +// Anthropic implements cryden's ai.LLMProvider (and widget.Composer) +// against the Anthropic Messages API. +// +// It is deliberately thin. The interesting work — deciding whether a +// model's answer is safe to run — belongs to cryden, and duplicating any +// of it here would create a second place for the rules to drift. +type Anthropic struct { + client anthropic.Client + model string + // maxTokens bounds one response. The answer is a small JSON object, + // so this is a ceiling on cost rather than on usefulness. + maxTokens int +} + +// AnthropicConfig is what the settings table stores, unpacked into what +// this constructor needs. +type AnthropicConfig struct { + APIKey string + Model string + MaxTokens int +} + +// NewAnthropic builds a provider from a stored configuration. +func NewAnthropic(cfg AnthropicConfig, opts ...option.RequestOption) (*Anthropic, error) { + if strings.TrimSpace(cfg.APIKey) == "" { + return nil, ErrNoAPIKey + } + opts = append(opts, option.WithAPIKey(cfg.APIKey)) + + return &Anthropic{ + client: anthropic.NewClient(opts...), + model: cfg.Model, + maxTokens: cfg.MaxTokens, + }, nil +} + +var ( + _ crydenai.LLMProvider = (*Anthropic)(nil) +) + +// intentSchema is the JSON schema the model's answer is constrained to. +// +// This is the second lock on a door cryden already bolts. The enums below +// are built from cryden's own allowlists rather than restated, so a model +// that has been talked into asking for the password hash cannot even +// express it: the field is not in the schema, and the API enforces the +// schema, not the prompt. cryden would reject the intent anyway — this +// just means the rejection almost never has to happen. +// +// Built from cryden's maps rather than hardcoded on purpose. If the +// engine allowlists a new field tomorrow, this schema follows it with no +// edit here; if the engine ever *removes* one, a hardcoded copy would +// keep offering it. +func intentSchema() map[string]any { + entities := sortedKeys(crydenai.AllowedEntities) + + // group_by is a single string field, and the schema is flat — it has + // no way to say "this enum depends on the entity you chose". So the + // enum it offers is the union across every entity's allowed fields, + // which is the honest superset: cryden checks group_by against the + // chosen entity's own list and rejects a mismatch. Narrowing here + // instead would mean duplicating cryden's per-entity rule in a form + // JSON Schema cannot express, and a stale copy of it would silently + // refuse a field the engine would have accepted. + groupable := map[string]bool{} + for _, fields := range crydenai.AllowedFields { + for field := range fields { + groupable[field] = true + } + } + + return map[string]any{ + "type": "object", + "properties": map[string]any{ + "entity": enumOf(entities), + "filters": map[string]any{ + "type": "array", + "items": map[string]any{ + "type": "object", + "properties": map[string]any{ + "field": map[string]any{"type": "string"}, + "operator": enumOf(sortedKeys(crydenai.AllowedOperators)), + "value": map[string]any{"type": "string"}, + }, + "required": []string{"field", "operator", "value"}, + "additionalProperties": false, + }, + }, + "aggregate": enumOf([]string{"", "count", "group_by"}), + "group_by": enumOf(sortedKeys(groupable)), + "limit": map[string]any{"type": "integer"}, + }, + "required": []string{"entity", "filters", "aggregate", "limit"}, + "additionalProperties": false, + } +} + +// systemPrompt is the whole of this repo's prompt. It is short because +// the schema does the constraining: the prompt's job is to say what the +// fields mean, not to enumerate what is allowed, and a prompt that listed +// the allowlist would be one more copy of it to keep in sync. +const systemPrompt = `You translate an administrator's question about their user database into a structured query intent. + +The intent names one entity, any filters narrowing it, and how to present the result. Use "count" when the question asks how many, "group_by" when it asks for a breakdown, and the empty aggregate when it asks for the rows themselves. + +Only the fields the schema offers exist. If the question cannot be expressed with them, choose the closest entity and omit the filters you cannot express rather than inventing a field name.` + +// ParseQueryIntent asks the model for a QueryIntent. The returned intent +// is unvalidated: cryden's ai.ExecuteIntent checks it against the +// allowlist before anything runs, and this function must not be assumed +// to have done so. +func (p *Anthropic) ParseQueryIntent(ctx context.Context, naturalLanguage string) (crydenai.QueryIntent, error) { + response, err := p.client.Messages.New(ctx, anthropic.MessageNewParams{ + Model: anthropic.Model(p.model), + MaxTokens: int64(p.maxTokens), + System: []anthropic.TextBlockParam{{ + Text: systemPrompt, + // The prompt and the schema are fixed for the life of the + // process, so every request after the first reads this from + // cache instead of paying for it again. + CacheControl: anthropic.NewCacheControlEphemeralParam(), + }}, + OutputConfig: anthropic.OutputConfigParam{ + Format: anthropic.JSONOutputFormatParam{Schema: intentSchema()}, + }, + Messages: []anthropic.MessageParam{ + anthropic.NewUserMessage(anthropic.NewTextBlock(naturalLanguage)), + }, + }) + if err != nil { + return crydenai.QueryIntent{}, fmt.Errorf("aiprovider: asking the model: %w", err) + } + + // Checked before the content is read, because a refusal carries no + // usable text and reading it first would report a refusal as a parse + // failure — sending an operator to look at their schema when the + // model simply declined the question. + if response.StopReason == anthropic.StopReasonRefusal { + return crydenai.QueryIntent{}, fmt.Errorf("%w: the model declined to answer (%s)", + ErrUnexpectedAnswer, response.StopDetails.Category) + } + + text := firstText(response.Content) + if text == "" { + return crydenai.QueryIntent{}, fmt.Errorf("%w: the response carried no text (stop reason %q)", + ErrUnexpectedAnswer, response.StopReason) + } + + var payload queryIntentPayload + if err := json.Unmarshal([]byte(text), &payload); err != nil { + return crydenai.QueryIntent{}, fmt.Errorf("%w: %v", ErrUnexpectedAnswer, err) + } + + intent := crydenai.QueryIntent{ + Entity: payload.Entity, + Aggregate: payload.Aggregate, + GroupBy: payload.GroupBy, + Limit: payload.Limit, + } + for _, f := range payload.Filters { + intent.Filters = append(intent.Filters, crydenai.QueryFilter{ + Field: f.Field, + Operator: f.Operator, + Value: f.Value, + }) + } + return intent, nil +} + +// queryIntentPayload mirrors cryden's ai.QueryIntent as JSON. A separate +// type rather than unmarshalling into ai.QueryIntent directly, because +// the wire shape and the engine's own struct are allowed to differ — +// QueryFilter is a struct here and an element of a slice there — and +// because a named type is where the JSON tags can be documented. +type queryIntentPayload struct { + Entity string `json:"entity"` + Filters []queryFilterPayload `json:"filters"` + Aggregate string `json:"aggregate"` + GroupBy string `json:"group_by"` + Limit int `json:"limit"` +} + +type queryFilterPayload struct { + Field string `json:"field"` + Operator string `json:"operator"` + Value string `json:"value"` +} + +// ComposeAnswer implements widget.Composer: it turns an already-validated, +// already-owner-scoped result into a sentence for an end user. +// +// The result it is given has been scoped to one identity by cryden's +// widget.Ask before it arrives — this function never sees another user's +// rows, and does not need to know that scoping exists. That is why it can +// be a plain presentation call with nothing to check. +func (p *Anthropic) ComposeAnswer(ctx context.Context, question string, result crydenai.QueryResult) (string, error) { + response, err := p.client.Messages.New(ctx, anthropic.MessageNewParams{ + Model: anthropic.Model(p.model), + MaxTokens: int64(p.maxTokens), + System: []anthropic.TextBlockParam{{ + Text: "You answer a user's question using only the rows provided. If the rows do not answer it, say so plainly. Never mention SQL, tables or column names.", + CacheControl: anthropic.NewCacheControlEphemeralParam(), + }}, + Messages: []anthropic.MessageParam{ + anthropic.NewUserMessage( + anthropic.NewTextBlock("Question: "+question), + anthropic.NewTextBlock("Rows:\n"+renderRows(result)), + ), + }, + }) + if err != nil { + return "", fmt.Errorf("aiprovider: composing an answer: %w", err) + } + if response.StopReason == anthropic.StopReasonRefusal { + return "", fmt.Errorf("%w: the model declined to answer (%s)", + ErrUnexpectedAnswer, response.StopDetails.Category) + } + return firstText(response.Content), nil +} + +// renderRows is a compact, positional rendering of a QueryResult for the +// composer. Headers once, then one line per row — the model is reading +// this, so a repeated header would be noise it might mistake for data. +func renderRows(result crydenai.QueryResult) string { + var b strings.Builder + b.WriteString(strings.Join(result.Columns, ", ")) + for _, row := range result.Rows { + b.WriteString("\n") + b.WriteString(strings.Join(row, ", ")) + } + return b.String() +} + +// firstText returns the first text block's content, or "". A response can +// carry thinking blocks before the text one, so this walks rather than +// indexing. +func firstText(blocks []anthropic.ContentBlockUnion) string { + for _, block := range blocks { + if text, ok := block.AsAny().(anthropic.TextBlock); ok { + return text.Text + } + } + return "" +} diff --git a/aiprovider/anthropic_test.go b/aiprovider/anthropic_test.go new file mode 100644 index 0000000..9c320d9 --- /dev/null +++ b/aiprovider/anthropic_test.go @@ -0,0 +1,331 @@ +package aiprovider + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "io" + "net/http" + "net/http/httptest" + "sort" + "strings" + "testing" + + "github.com/anthropics/anthropic-sdk-go/option" + + crydenai "github.com/crydensync/cryden/v2/ai" +) + +// The provider is tested against a local HTTP server that answers in the +// Messages API's wire shape, rather than against the live API. That is +// not a shortcut around testing the interesting part: everything this +// package does — building the request, reading the response, turning it +// into a QueryIntent — happens on this side of the socket, and a fake +// server is the only way to drive the failure paths (a refusal, a +// truncated answer, an HTTP error) on purpose rather than by luck. +// +// What is NOT covered: the real API's behaviour. Whether the model +// answers well, and whether the output schema is accepted as written, is +// only knowable against the live service. PROGRESS.md says so. +type fakeAPI struct { + *httptest.Server + // lastBody is the decoded request body of the most recent call, so a + // test can assert what was actually sent rather than only what came + // back. + lastBody map[string]any + // calls counts requests, so a test can prove a refusal was not + // retried into a second charge. + calls int +} + +// newFakeAPI answers every request with the given assistant text. +func newFakeAPI(t *testing.T, answer string) *fakeAPI { + t.Helper() + f := &fakeAPI{} + f.Server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + raw, _ := io.ReadAll(r.Body) + var body map[string]any + _ = json.Unmarshal(raw, &body) + f.lastBody = body + f.calls++ + + w.Header().Set("Content-Type", "application/json") + fmt.Fprintf(w, `{ + "id": "msg_test", "type": "message", "role": "assistant", + "model": "claude-opus-5", "stop_reason": "end_turn", + "content": [{"type": "text", "text": %s}], + "usage": {"input_tokens": 1, "output_tokens": 1} + }`, mustJSON(t, answer)) + })) + t.Cleanup(f.Close) + return f +} + +// newFakeAPIResponse answers with a complete response body, for the cases +// where the shape matters more than the text. +func newFakeAPIResponse(t *testing.T, body string) *fakeAPI { + t.Helper() + f := &fakeAPI{} + f.Server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + raw, _ := io.ReadAll(r.Body) + var decoded map[string]any + _ = json.Unmarshal(raw, &decoded) + f.lastBody = decoded + f.calls++ + + w.Header().Set("Content-Type", "application/json") + _, _ = io.WriteString(w, body) + })) + t.Cleanup(f.Close) + return f +} + +func mustJSON(t *testing.T, v any) string { + t.Helper() + raw, err := json.Marshal(v) + if err != nil { + t.Fatalf("marshalling %v: %v", v, err) + } + return string(raw) +} + +func (f *fakeAPI) provider(t *testing.T) *Anthropic { + t.Helper() + p, err := NewAnthropic( + AnthropicConfig{APIKey: "test-key", Model: "claude-opus-5", MaxTokens: 1024}, + option.WithBaseURL(f.URL), + ) + if err != nil { + t.Fatalf("NewAnthropic: %v", err) + } + return p +} + +func TestNewAnthropicRefusesAnEmptyKey(t *testing.T) { + for _, key := range []string{"", " "} { + if _, err := NewAnthropic(AnthropicConfig{APIKey: key}); !errors.Is(err, ErrNoAPIKey) { + t.Errorf("NewAnthropic(APIKey %q) = %v, want ErrNoAPIKey", key, err) + } + } +} + +func TestParseQueryIntentReadsTheModelsAnswer(t *testing.T) { + api := newFakeAPI(t, `{"entity":"users","filters":[{"field":"email","operator":"=","value":"dana@example.com"}],"aggregate":"","group_by":"","limit":10}`) + + intent, err := api.provider(t).ParseQueryIntent(context.Background(), "show me dana") + if err != nil { + t.Fatalf("ParseQueryIntent: %v", err) + } + if intent.Entity != "users" { + t.Errorf("Entity = %q, want %q", intent.Entity, "users") + } + if len(intent.Filters) != 1 { + t.Fatalf("Filters = %+v, want exactly one", intent.Filters) + } + got := intent.Filters[0] + if got.Field != "email" || got.Operator != "=" || got.Value != "dana@example.com" { + t.Errorf("Filters[0] = %+v, want the email filter the model returned", got) + } + if intent.Limit != 10 { + t.Errorf("Limit = %d, want 10", intent.Limit) + } +} + +// The schema is the second lock on the door: a model that has been argued +// into asking for a password hash must not even be able to express it. +func TestIntentSchemaOffersOnlyWhatCrydenWouldAccept(t *testing.T) { + schema := intentSchema() + props, ok := schema["properties"].(map[string]any) + if !ok { + t.Fatalf("schema has no properties object: %+v", schema) + } + + entityEnum := enumValues(t, props["entity"]) + for entity := range crydenai.AllowedEntities { + if !contains(entityEnum, entity) { + t.Errorf("schema omits the allowlisted entity %q", entity) + } + } + if len(entityEnum) != len(crydenai.AllowedEntities) { + t.Errorf("entity enum = %v, want exactly cryden's allowlist %v", entityEnum, crydenai.AllowedEntities) + } + + // The columns that must never be reachable through this path. cryden + // leaves them out of AllowedFields; this asserts the schema does too, + // because the schema is what the model is physically able to emit. + groupBy := enumValues(t, props["group_by"]) + for _, forbidden := range []string{"password_hash", "token_hash", "PasswordHash", "TokenHash"} { + if contains(groupBy, forbidden) { + t.Errorf("group_by enum offers %q, which cryden deliberately never allowlists", forbidden) + } + } + + operatorEnum := enumValues(t, filterItemProps(t, props)["operator"]) + for operator := range crydenai.AllowedOperators { + if !contains(operatorEnum, operator) { + t.Errorf("schema omits the allowlisted operator %q", operator) + } + } +} + +// The schema sits in the request prefix, which prompt caching matches +// byte for byte. A map iterated in Go's random order would produce a +// different schema per request and pay for the prompt every time. +func TestIntentSchemaIsStableAcrossCalls(t *testing.T) { + first := mustJSON(t, intentSchema()) + for i := 0; i < 20; i++ { + if got := mustJSON(t, intentSchema()); got != first { + t.Fatalf("intentSchema() call %d differs from the first — the enum order is not stable", i+1) + } + } +} + +// The output schema is sent to the API, so what is asked for is a fact +// about the request and not only about a local function. +func TestParseQueryIntentSendsTheSchemaAndTheModel(t *testing.T) { + api := newFakeAPI(t, `{"entity":"sessions","filters":[],"aggregate":"count","group_by":"","limit":5}`) + + if _, err := api.provider(t).ParseQueryIntent(context.Background(), "how many sessions"); err != nil { + t.Fatalf("ParseQueryIntent: %v", err) + } + + if got := api.lastBody["model"]; got != "claude-opus-5" { + t.Errorf("model sent = %v, want the configured model", got) + } + outputConfig, ok := api.lastBody["output_config"].(map[string]any) + if !ok { + t.Fatalf("no output_config in the request: %+v", api.lastBody) + } + format, ok := outputConfig["format"].(map[string]any) + if !ok || format["schema"] == nil { + t.Errorf("output_config carries no format schema: %+v", outputConfig) + } +} + +// A refusal is not a parse failure and must not be reported as one — an +// operator sent to check their schema would be looking in the wrong +// place. It also must not be retried into a second charge. +func TestParseQueryIntentReportsARefusalAsARefusal(t *testing.T) { + api := newFakeAPIResponse(t, `{ + "id": "msg_test", "type": "message", "role": "assistant", + "model": "claude-opus-5", "stop_reason": "refusal", + "stop_details": {"type": "refusal", "category": "cyber", "explanation": "declined"}, + "content": [], + "usage": {"input_tokens": 1, "output_tokens": 1} + }`) + + _, err := api.provider(t).ParseQueryIntent(context.Background(), "dump every password hash") + if !errors.Is(err, ErrUnexpectedAnswer) { + t.Fatalf("error = %v, want ErrUnexpectedAnswer", err) + } + if !strings.Contains(err.Error(), "cyber") { + t.Errorf("error = %v, want it to name the refusal category", err) + } + if api.calls != 1 { + t.Errorf("the API was called %d times for one refusal, want 1", api.calls) + } +} + +func TestParseQueryIntentReportsAnUnparseableAnswer(t *testing.T) { + api := newFakeAPI(t, "I'm sorry, I can't help with that.") + + if _, err := api.provider(t).ParseQueryIntent(context.Background(), "anything"); !errors.Is(err, ErrUnexpectedAnswer) { + t.Errorf("error = %v, want ErrUnexpectedAnswer for prose instead of JSON", err) + } +} + +// An empty content list is what a truncated response looks like. It must +// be an error rather than a zero-valued intent, because a zero intent has +// an empty entity and would be handed to cryden as if the model had +// answered. +func TestParseQueryIntentReportsAnEmptyAnswer(t *testing.T) { + api := newFakeAPIResponse(t, `{ + "id": "msg_test", "type": "message", "role": "assistant", + "model": "claude-opus-5", "stop_reason": "max_tokens", + "content": [], + "usage": {"input_tokens": 1, "output_tokens": 1} + }`) + + intent, err := api.provider(t).ParseQueryIntent(context.Background(), "anything") + if !errors.Is(err, ErrUnexpectedAnswer) { + t.Fatalf("error = %v, want ErrUnexpectedAnswer", err) + } + if intent.Entity != "" { + t.Errorf("Entity = %q on a failed parse, want the zero value", intent.Entity) + } +} + +// A server error is reported as itself rather than as a model problem: it +// says nothing about the question and everything about the deployment. +func TestParseQueryIntentReportsATransportFailure(t *testing.T) { + api := &fakeAPI{} + api.Server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + http.Error(w, `{"type":"error","error":{"type":"api_error","message":"boom"}}`, http.StatusInternalServerError) + })) + t.Cleanup(api.Close) + + _, err := api.provider(t).ParseQueryIntent(context.Background(), "anything") + if err == nil { + t.Fatal("ParseQueryIntent against a 500 returned no error") + } + if errors.Is(err, ErrUnexpectedAnswer) { + t.Errorf("error = %v, want a transport failure rather than ErrUnexpectedAnswer", err) + } +} + +func TestComposeAnswerReturnsTheModelsProse(t *testing.T) { + api := newFakeAPI(t, "You signed in from three devices this week.") + + text, err := api.provider(t).ComposeAnswer(context.Background(), "where did I sign in from?", + crydenai.QueryResult{Columns: []string{"ip"}, Rows: [][]string{{"203.0.113.1"}}}) + if err != nil { + t.Fatalf("ComposeAnswer: %v", err) + } + if text != "You signed in from three devices this week." { + t.Errorf("ComposeAnswer = %q, want the model's text", text) + } +} + +// enumValues pulls the values out of a JSON Schema enum node. +func enumValues(t *testing.T, node any) []string { + t.Helper() + object, ok := node.(map[string]any) + if !ok { + t.Fatalf("expected an enum object, got %T (%v)", node, node) + } + raw, ok := object["enum"].([]string) + if !ok { + t.Fatalf("expected a string enum, got %T (%v)", object["enum"], object["enum"]) + } + out := append([]string(nil), raw...) + sort.Strings(out) + return out +} + +// filterItemProps reaches into the filters array's item schema. +func filterItemProps(t *testing.T, props map[string]any) map[string]any { + t.Helper() + filters, ok := props["filters"].(map[string]any) + if !ok { + t.Fatalf("schema has no filters object: %+v", props) + } + items, ok := filters["items"].(map[string]any) + if !ok { + t.Fatalf("filters has no items schema: %+v", filters) + } + itemProps, ok := items["properties"].(map[string]any) + if !ok { + t.Fatalf("filter items have no properties: %+v", items) + } + return itemProps +} + +func contains(haystack []string, needle string) bool { + for _, h := range haystack { + if h == needle { + return true + } + } + return false +} diff --git a/aiprovider/query.go b/aiprovider/query.go new file mode 100644 index 0000000..9de2097 --- /dev/null +++ b/aiprovider/query.go @@ -0,0 +1,358 @@ +package aiprovider + +import ( + "context" + "database/sql" + "errors" + "fmt" + "strings" + "time" + + "github.com/lib/pq" + + crydenai "github.com/crydensync/cryden/v2/ai" +) + +// ErrNotReadOnly means the supplied role CAN write, so it must not back +// the AI query surface. +var ErrNotReadOnly = errors.New("aiprovider: the database role is not read-only") + +// ErrCannotVerifyReadOnly means the check could not reach a conclusion. +// Separate from ErrNotReadOnly on purpose: "this role can write" and "we +// could not find out" call for different words, and the second one is +// almost always a connection problem the operator can fix. +var ErrCannotVerifyReadOnly = errors.New("aiprovider: could not verify the database role is read-only") + +// readOnlyProbeTimeout bounds the whole check. It is short because this +// runs inside an HTTP request from a settings form — a check that hangs +// for a minute would look like a broken console, and an operator +// configuring a second database expects a few seconds at most. +const readOnlyProbeTimeout = 10 * time.Second + +// probeTable is the scratch table the write attempt targets. It lives in +// pg_temp, the session's own temporary schema, which is what makes this +// check safe to run: the table exists only for the life of this +// connection, is invisible to every other session, and is dropped by +// Postgres when the connection closes — so a failed CREATE leaves nothing +// behind for the operator to clean up. +// +// It is a single-column table with a single row. The point is not the +// data, it is whether the server says yes. +const ( + probeCreate = `CREATE TEMP TABLE cryden_readonly_probe (id int)` + probeInsert = `INSERT INTO cryden_readonly_probe (id) VALUES (1)` +) + +// CheckReadOnly verifies that dsn names a role which cannot write. +// +// This is the check cryden's ai.QueryableStore interface asks for by name: +// "MUST use a read-only Postgres role for this connection — that's a real +// credential-level guarantee, not just a promise made in code, so a bug +// in validation still can't cause a write." The allowlist in ai.validate.go +// is the first line of defence; this is the line that holds when the +// first one has a bug, because a role that cannot INSERT cannot INSERT +// whatever a query builder does with its input. +// +// Which is also why this is checked by *attempting a write and confirming +// it is rejected*, rather than by reading the role's attributes. Trusting +// a checkbox, or pg_roles.rolsuper, or the presence of "readonly" in a +// connection parameter would all be trusting a claim. Only the server's +// refusal is evidence — and it is evidence about the actual role, on the +// actual database, through the actual credentials, which no amount of +// reading metadata can substitute for. +// +// The three outcomes are deliberately distinct: +// +// - the write is refused -> nil, the role is read-only +// - the write succeeds -> ErrNotReadOnly +// - anything else -> ErrCannotVerifyReadOnly +// +// The third is not a pass. A connection that never opened, a timeout, a +// missing table privilege that fails the CREATE for a reason other than +// read-onlyness — none of those prove anything, and treating them as +// success would make this check pass exactly when it is least able to +// tell. +func CheckReadOnly(ctx context.Context, dsn string) error { + ctx, cancel := context.WithTimeout(ctx, readOnlyProbeTimeout) + defer cancel() + + db, err := sql.Open("postgres", dsn) + if err != nil { + return fmt.Errorf("%w: opening the connection: %v", ErrCannotVerifyReadOnly, err) + } + defer db.Close() + + // One connection, used for both statements. pg_temp is per-session, + // so a pool that handed the CREATE and the INSERT to different + // connections would have the INSERT fail on a missing table — a + // refusal that looks like proof of read-onlyness and is not. + db.SetMaxOpenConns(1) + db.SetMaxIdleConns(1) + + if err := db.PingContext(ctx); err != nil { + return fmt.Errorf("%w: connecting: %v", ErrCannotVerifyReadOnly, err) + } + + // A read that must work, before any write is attempted. Without it, a + // role with no rights at all would fail the CREATE below and be + // reported as read-only — which is the right answer by accident, on a + // connection that cannot serve the feature either. + if _, err := db.ExecContext(ctx, `SELECT 1`); err != nil { + return fmt.Errorf("%w: the connection cannot run a query at all: %v", ErrCannotVerifyReadOnly, err) + } + + if _, err := db.ExecContext(ctx, probeCreate); err != nil { + // The CREATE failing is the expected outcome for a role that + // cannot write, and it is also what a dozen unrelated problems + // look like. Postgres distinguishes them: 42501 is + // insufficient_privilege, which is the server saying "this role + // may not do that". Anything else is reported as unverifiable + // rather than assumed to be a refusal. + if isInsufficientPrivilege(err) { + return nil + } + return fmt.Errorf("%w: creating the probe table: %v", ErrCannotVerifyReadOnly, err) + } + + // The role could create a table. It is not read-only, and the INSERT + // is not needed to know that — but running it keeps the failure + // message specific about what succeeded, which is what an operator + // needs to go and fix the grant. + if _, err := db.ExecContext(ctx, probeInsert); err != nil { + return fmt.Errorf("%w: the role may create tables (INSERT failed separately: %v)", ErrNotReadOnly, err) + } + return fmt.Errorf("%w: the role both created a table and inserted a row", ErrNotReadOnly) +} + +// isInsufficientPrivilege reports whether err is Postgres' own "this role +// may not do that" — SQLSTATE 42501. +// +// Checked by code rather than by matching the message, because the +// message is localized and reworded between major versions while the code +// is not, and this is the branch that decides whether a connection is +// accepted. +func isInsufficientPrivilege(err error) bool { + var pqErr *pq.Error + if errors.As(err, &pqErr) { + return pqErr.Code == "42501" + } + // lib/pq returns the parsed error for anything the server answers, + // so a non-pq error here means the statement never reached Postgres. + // Reported as unverifiable rather than as a refusal, which is what + // the caller does with a false return. + return false +} + +// PostgresSnapshot implements cryden's ai.QueryableStore over a +// connection that CheckReadOnly has already accepted. +// +// It runs an already-validated QueryIntent. "Already-validated" is not a +// hope: cryden's ai.ExecuteIntent runs validateIntent before it calls +// RunSafeQuery at all, and widget.Ask runs it too — there is no path to +// this method that skips that. What this type adds is that the statement +// it builds is assembled from cryden's own EntityColumns list rather than +// from anything on the intent, so an entity that somehow got past +// validation still cannot put text of its own into the SQL. +type PostgresSnapshot struct { + db *sql.DB + maxRows int +} + +// NewPostgresSnapshot opens the read-only connection. It does NOT itself +// check that the role is read-only — that is CheckReadOnly's job and it +// belongs to the settings write, where the operator is present to be told +// about a failure. Opening here is deliberately cheap so that a +// deployment whose stored connection has since gone bad reports a query +// error rather than failing to start. +func NewPostgresSnapshot(dsn string, maxRows int) (*PostgresSnapshot, error) { + db, err := sql.Open("postgres", dsn) + if err != nil { + return nil, fmt.Errorf("aiprovider: opening the read-only connection: %w", err) + } + if maxRows < 1 { + maxRows = crydenai.DefaultLimit + } + if maxRows > crydenai.MaxLimit { + maxRows = crydenai.MaxLimit + } + return &PostgresSnapshot{db: db, maxRows: maxRows}, nil +} + +// Close releases the pool. +func (s *PostgresSnapshot) Close() error { return s.db.Close() } + +var _ crydenai.QueryableStore = (*PostgresSnapshot)(nil) + +// RunSafeQuery executes an intent. +// +// Every value that reaches the SQL text comes from cryden's own +// EntityColumns / AllowedFields maps. Filter *values* — the only part +// that originates with a user — are bind parameters, never interpolated. +// Column and operator names cannot be bound in SQL, which is exactly why +// they are taken from a fixed map on this side rather than trusted from +// the intent: a name that is not in the map is refused, not quoted. +func (s *PostgresSnapshot) RunSafeQuery(ctx context.Context, intent crydenai.QueryIntent) (crydenai.QueryResult, error) { + columns, ok := crydenai.EntityColumns[intent.Entity] + if !ok { + return crydenai.QueryResult{}, fmt.Errorf("aiprovider: unknown entity %q", intent.Entity) + } + + // The intent's own Limit was already defaulted and clamped by + // ai.ExecuteIntent; this is the deployment's own ceiling on top of + // the engine's, so an operator can make the AI surface cheaper than + // cryden's maximum without changing the engine. + limit := intent.Limit + if limit <= 0 || limit > s.maxRows { + limit = s.maxRows + } + + var ( + where []string + args []any + ) + for _, filter := range intent.Filters { + if err := checkFilter(intent.Entity, filter); err != nil { + return crydenai.QueryResult{}, err + } + args = append(args, filterArgument(filter.Operator, filter.Value)) + where = append(where, fmt.Sprintf("%s %s $%d", filter.Field, sqlOperator(filter.Operator), len(args))) + } + + statement := buildStatement(intent, columns, where, limit) + rows, err := s.db.QueryContext(ctx, statement, args...) + if err != nil { + return crydenai.QueryResult{}, fmt.Errorf("aiprovider: running the query: %w", err) + } + defer rows.Close() + + return scanResult(rows) +} + +// checkFilter refuses a filter whose field or operator is not in cryden's +// allowlist for that entity. +// +// Both names are checked here as well as in the engine. That is not +// redundancy for its own sake: SQL cannot bind a column or an operator +// name, so those two are the only parts of this query that are spliced +// into text rather than passed as parameters, and a second check at the +// point of assembly is what makes "the statement can only contain names +// from a fixed map" a property of this function rather than a claim about +// its callers. +func checkFilter(entity string, filter crydenai.QueryFilter) error { + if !crydenai.AllowedFields[entity][filter.Field] { + return fmt.Errorf("aiprovider: field %q is not allowed on %q", filter.Field, entity) + } + if !crydenai.AllowedOperators[filter.Operator] { + return fmt.Errorf("aiprovider: operator %q is not allowed", filter.Operator) + } + return nil +} + +// buildStatement assembles the SELECT. Every piece spliced into the text +// is a name drawn from cryden's maps — see RunSafeQuery — so the only +// thing a caller influences is the shape, never the vocabulary. +func buildStatement(intent crydenai.QueryIntent, columns, where []string, limit int) string { + var b strings.Builder + + switch intent.Aggregate { + case "count": + b.WriteString("SELECT count(*) FROM ") + b.WriteString(intent.Entity) + case "group_by": + b.WriteString("SELECT ") + b.WriteString(intent.GroupBy) + b.WriteString(", count(*) FROM ") + b.WriteString(intent.Entity) + default: + b.WriteString("SELECT ") + b.WriteString(strings.Join(columns, ", ")) + b.WriteString(" FROM ") + b.WriteString(intent.Entity) + } + + if len(where) > 0 { + b.WriteString(" WHERE ") + b.WriteString(strings.Join(where, " AND ")) + } + if intent.Aggregate == "group_by" { + b.WriteString(" GROUP BY ") + b.WriteString(intent.GroupBy) + } + fmt.Fprintf(&b, " LIMIT %d", limit) + return b.String() +} + +// sqlOperator maps cryden's operator vocabulary onto SQL. "contains" is +// the one that is not a plain symbol: it becomes a LIKE. +func sqlOperator(operator string) string { + if operator == "contains" { + return "LIKE" + } + return operator +} + +// filterArgument prepares a filter's value for its place in the query. +// +// "contains" is the only operator that needs anything done to its value: +// LIKE's wildcards are part of the pattern, so a caller writing +// "contains: a%" would otherwise get substring semantics they did not ask +// for and probably did not intend. The wildcards are added here, around +// the whole value, so the value is always a literal. +// +// This is not a safety measure — the value is a bind parameter either +// way, so neither form can reach the SQL text. It is about the operator +// meaning what it says. +func filterArgument(operator, value string) string { + if operator == "contains" { + // Escaped so a literal % or _ in the question stays literal: + // the default LIKE escape character is a backslash. + escaped := strings.NewReplacer(`\`, `\\`, `%`, `\%`, `_`, `\_`).Replace(value) + return "%" + escaped + "%" + } + return value +} + +func scanResult(rows *sql.Rows) (crydenai.QueryResult, error) { + // The header comes from the query rather than from the entity + // definition, because an aggregate's columns are not the entity's + // own — a count returns one column that no entity lists. + names, err := rows.Columns() + if err != nil { + return crydenai.QueryResult{}, err + } + + result := crydenai.QueryResult{Columns: names, Rows: [][]string{}} + for rows.Next() { + cells := make([]any, len(names)) + pointers := make([]any, len(names)) + for i := range cells { + pointers[i] = &cells[i] + } + if err := rows.Scan(pointers...); err != nil { + return crydenai.QueryResult{}, err + } + row := make([]string, len(names)) + for i, cell := range cells { + row[i] = renderCell(cell) + } + result.Rows = append(result.Rows, row) + } + return result, rows.Err() +} + +// renderCell turns a scanned value into the string form ai.QueryResult +// promises. Bytes become a string, a time keeps RFC 3339, and a NULL +// becomes empty rather than the word "NULL" — an empty cell is what a +// table shows and what a model reads as "nothing here". +func renderCell(cell any) string { + switch value := cell.(type) { + case nil: + return "" + case []byte: + return string(value) + case time.Time: + return value.Format(time.RFC3339) + default: + return fmt.Sprintf("%v", value) + } +} diff --git a/aiprovider/query_test.go b/aiprovider/query_test.go new file mode 100644 index 0000000..701d034 --- /dev/null +++ b/aiprovider/query_test.go @@ -0,0 +1,198 @@ +package aiprovider + +import ( + "errors" + "strings" + "testing" + + crydenai "github.com/crydensync/cryden/v2/ai" +) + +// The statement builder is where the two spliced names — a column and an +// operator — become SQL text. Everything else in a query is a bind +// parameter. So this is the file that has to be sure about them. +func TestBuildStatementUsesOnlyAllowlistedNames(t *testing.T) { + columns, ok := crydenai.EntityColumns["users"] + if !ok { + t.Fatal("cryden no longer defines EntityColumns for users") + } + + statement := buildStatement( + crydenai.QueryIntent{Entity: "users", Aggregate: "", Limit: 25}, + columns, + nil, + 25, + ) + + want := "SELECT " + strings.Join(columns, ", ") + " FROM users LIMIT 25" + if statement != want { + t.Errorf("statement =\n %s\nwant\n %s", statement, want) + } +} + +func TestBuildStatementShapesAnAggregate(t *testing.T) { + t.Run("count", func(t *testing.T) { + got := buildStatement(crydenai.QueryIntent{Entity: "audit_events", Aggregate: "count", Limit: 5}, nil, nil, 5) + if got != "SELECT count(*) FROM audit_events LIMIT 5" { + t.Errorf("statement = %q", got) + } + }) + + t.Run("group_by", func(t *testing.T) { + got := buildStatement( + crydenai.QueryIntent{Entity: "audit_events", Aggregate: "group_by", GroupBy: "type", Limit: 5}, + nil, nil, 5, + ) + want := "SELECT type, count(*) FROM audit_events GROUP BY type LIMIT 5" + if got != want { + t.Errorf("statement = %q, want %q", got, want) + } + }) +} + +func TestBuildStatementBindsFilterValuesRatherThanInlining(t *testing.T) { + got := buildStatement( + crydenai.QueryIntent{Entity: "users", Limit: 10}, + nil, + []string{"email = $1", "created_at > $2"}, + 10, + ) + if !strings.Contains(got, "WHERE email = $1 AND created_at > $2") { + t.Errorf("statement = %q, want the filters joined with AND and left as placeholders", got) + } +} + +// A column or operator that is not in cryden's allowlist must be refused +// before it can be spliced into a statement. Asserted with values that +// would be an injection if they ever reached the text. +func TestCheckFilterRefusesAnythingNotAllowlisted(t *testing.T) { + cases := []struct { + name string + entity string + filter crydenai.QueryFilter + }{ + {"unknown field", "users", crydenai.QueryFilter{Field: "password_hash", Operator: "=", Value: "x"}}, + {"injected field", "users", crydenai.QueryFilter{Field: "email; DROP TABLE users", Operator: "=", Value: "x"}}, + {"unknown operator", "users", crydenai.QueryFilter{Field: "email", Operator: "OR 1=1 --", Value: "x"}}, + {"field from another entity", "users", crydenai.QueryFilter{Field: "user_agent", Operator: "=", Value: "x"}}, + {"unknown entity", "secrets", crydenai.QueryFilter{Field: "id", Operator: "=", Value: "x"}}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if err := checkFilter(tc.entity, tc.filter); err == nil { + t.Errorf("checkFilter(%q, %+v) was allowed, want a refusal", tc.entity, tc.filter) + } + }) + } +} + +func TestCheckFilterAllowsWhatCrydenAllows(t *testing.T) { + // Every field cryden allowlists on every entity has to pass here too, + // or this repo would refuse queries the engine considers safe. + for entity, fields := range crydenai.AllowedFields { + for field := range fields { + for operator := range crydenai.AllowedOperators { + if err := checkFilter(entity, crydenai.QueryFilter{Field: field, Operator: operator}); err != nil { + t.Errorf("checkFilter(%q, %s %s) = %v, want nil", entity, field, operator, err) + } + } + } + } +} + +// "contains" means substring, and the wildcards that make it one are +// added here rather than taken from the caller's value. +func TestFilterArgumentWrapsContainsAndEscapesItsWildcards(t *testing.T) { + cases := []struct { + value string + want string + }{ + {"dana", "%dana%"}, + {"", "%%"}, + // A caller's own % must stay a literal percent rather than + // becoming a wildcard they did not ask for. + {"100%", `%100\%%`}, + {"a_b", `%a\_b%`}, + {`back\slash`, `%back\\slash%`}, + } + for _, tc := range cases { + if got := filterArgument("contains", tc.value); got != tc.want { + t.Errorf("filterArgument(contains, %q) = %q, want %q", tc.value, got, tc.want) + } + } + + // Every other operator passes its value through untouched — an "=" + // that mangled its value would silently match nothing. + for _, operator := range []string{"=", ">", "<"} { + if got := filterArgument(operator, "100%"); got != "100%" { + t.Errorf("filterArgument(%q, ...) = %q, want the value unchanged", operator, got) + } + } +} + +func TestSqlOperatorMapsOnlyContains(t *testing.T) { + if got := sqlOperator("contains"); got != "LIKE" { + t.Errorf("sqlOperator(contains) = %q, want LIKE", got) + } + for _, operator := range []string{"=", ">", "<"} { + if got := sqlOperator(operator); got != operator { + t.Errorf("sqlOperator(%q) = %q, want it unchanged", operator, got) + } + } +} + +func TestRenderCell(t *testing.T) { + cases := []struct { + name string + cell any + want string + }{ + // NULL is an empty cell. The word "NULL" would be read by a model + // as the four-character string it looks like. + {"null", nil, ""}, + {"bytes", []byte("dana@example.com"), "dana@example.com"}, + {"string", "already a string", "already a string"}, + {"int", 42, "42"}, + {"bool", true, "true"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := renderCell(tc.cell); got != tc.want { + t.Errorf("renderCell(%v) = %q, want %q", tc.cell, got, tc.want) + } + }) + } +} + +// Only Postgres' own insufficient_privilege is read as "the role is +// read-only". Everything else has to fall through to unverifiable, or a +// connection failure would be accepted as proof. +func TestIsInsufficientPrivilegeAcceptsOnly42501(t *testing.T) { + if isInsufficientPrivilege(nil) { + t.Error("a nil error was read as insufficient privilege") + } + if isInsufficientPrivilege(errors.New("dial tcp: connection refused")) { + t.Error("a transport error was read as insufficient privilege") + } + if isInsufficientPrivilege(errors.New(`pq: permission denied for table users`)) { + t.Error("a plain error was read as insufficient privilege — only a parsed SQLSTATE may count") + } +} + +// The probe statements are the evidence this whole check rests on, so +// they must be a write and must not touch anything an operator would have +// to clean up afterwards. +func TestProbeStatementsWriteOnlyToATemporaryTable(t *testing.T) { + if !strings.HasPrefix(strings.ToUpper(probeCreate), "CREATE TEMP TABLE") { + t.Errorf("probeCreate = %q, want a TEMP table so a successful probe leaves nothing behind", probeCreate) + } + if !strings.HasPrefix(strings.ToUpper(probeInsert), "INSERT") { + t.Errorf("probeInsert = %q, want a genuine write", probeInsert) + } + for _, statement := range []string{probeCreate, probeInsert} { + if strings.Contains(strings.ToUpper(statement), "PG_CATALOG") || strings.Contains(strings.ToUpper(statement), "PUBLIC.") { + t.Errorf("probe statement %q touches a real schema", statement) + } + } +} diff --git a/aiprovider/schema.go b/aiprovider/schema.go new file mode 100644 index 0000000..9b31bdb --- /dev/null +++ b/aiprovider/schema.go @@ -0,0 +1,27 @@ +package aiprovider + +import "sort" + +// sortedKeys returns the keys of a string-keyed set in a stable order. +// +// The order matters and is not cosmetic. These feed JSON Schema enums +// that sit inside the request's system-prompt prefix, and prompt caching +// is a prefix match — a map iterated in Go's random order would produce a +// different schema on every request, invalidating the cache every time +// and paying full price for a prompt that never changed. +func sortedKeys(set map[string]bool) []string { + keys := make([]string, 0, len(set)) + for key := range set { + keys = append(keys, key) + } + sort.Strings(keys) + return keys +} + +// enumOf wraps values as a JSON Schema enum of strings. +func enumOf(values []string) map[string]any { + return map[string]any{ + "type": "string", + "enum": values, + } +} diff --git a/aiprovider/scoped.go b/aiprovider/scoped.go new file mode 100644 index 0000000..6a921cf --- /dev/null +++ b/aiprovider/scoped.go @@ -0,0 +1,83 @@ +package aiprovider + +import ( + "context" + "errors" + + crydenai "github.com/crydensync/cryden/v2/ai" +) + +// ErrEntityOutOfScope means the parsed intent named an entity the +// deployment has not made available to the ask-ai widget. +var ErrEntityOutOfScope = errors.New("aiprovider: entity is outside the configured ask-ai scope") + +// ScopedProvider narrows an ai.LLMProvider to a configured set of +// entities. +// +// It sits in front of the provider the widget uses, not in front of the +// admin one. cryden's widget.Ask already forces every intent to the +// calling end user's own rows — it discards whatever identity filter the +// model produced and substitutes the real one — and that is the security +// boundary, which this type does not touch or replace. What it adds is +// the layer above: which entities a deployment is willing to answer +// questions about at all, from a public-facing surface. +// +// The distinction matters because the two are decided by different +// people. cryden decides what can be scoped safely; the operator decides +// what this deployment offers. An operator who wants the widget to answer +// "when did I last log in" but not "what has been recorded against me" +// has no way to say so through cryden's fixed allowlist, and asking +// cryden to grow a per-host policy knob would be putting a host decision +// in the engine — the boundary CLAUDE.md draws. +// +// Wrapping ParseQueryIntent is the only place this can be done. By the +// time widget.Ask has an Answer, the query has already run, and the +// intent itself never leaves the package: Ask parses, scopes and executes +// in one call. Refusing at parse time is the one point where the entity +// is still visible to the host and nothing has been executed yet. +type ScopedProvider struct { + inner crydenai.LLMProvider + entities map[string]bool +} + +// NewScopedProvider wraps inner, refusing any entity not in entities. +// +// An empty entities set produces a provider that refuses everything, +// which is the correct reading of "no scope configured": a settings form +// that was never filled in should answer no questions rather than all of +// them. The caller is expected to check the widget's own Enabled flag +// before it gets this far; this is the second lock on the same door. +func NewScopedProvider(inner crydenai.LLMProvider, entities []string) *ScopedProvider { + allowed := make(map[string]bool, len(entities)) + for _, entity := range entities { + allowed[entity] = true + } + return &ScopedProvider{inner: inner, entities: allowed} +} + +var _ crydenai.LLMProvider = (*ScopedProvider)(nil) + +// ParseQueryIntent defers to the wrapped provider and then refuses an +// entity outside the configured scope. +// +// The refusal happens after the parse rather than before it, because the +// entity is the model's output and does not exist until then. That costs +// a model call on a question that will be rejected, which is the honest +// price: the alternative would be a second model call asking the model to +// classify its own question first, which is more expensive, less +// reliable, and still untrusted input. +// +// The error names neither the entity nor the scope. It reaches an end +// user through the widget, and telling them which entities this +// deployment does have configured would be describing the console's +// schema to whoever is typing questions at it. +func (p *ScopedProvider) ParseQueryIntent(ctx context.Context, naturalLanguage string) (crydenai.QueryIntent, error) { + intent, err := p.inner.ParseQueryIntent(ctx, naturalLanguage) + if err != nil { + return crydenai.QueryIntent{}, err + } + if !p.entities[intent.Entity] { + return crydenai.QueryIntent{}, ErrEntityOutOfScope + } + return intent, nil +} diff --git a/aiprovider/scoped_test.go b/aiprovider/scoped_test.go new file mode 100644 index 0000000..c2c082d --- /dev/null +++ b/aiprovider/scoped_test.go @@ -0,0 +1,102 @@ +package aiprovider + +import ( + "context" + "errors" + "strings" + "testing" + + crydenai "github.com/crydensync/cryden/v2/ai" +) + +// fixedProvider returns whatever intent it was built with, which is what +// makes it usable as the inner provider here: the test controls exactly +// what the model "produced" without any network call. +type fixedProvider struct { + intent crydenai.QueryIntent + err error + + calls int +} + +func (p *fixedProvider) ParseQueryIntent(context.Context, string) (crydenai.QueryIntent, error) { + p.calls++ + if p.err != nil { + return crydenai.QueryIntent{}, p.err + } + return p.intent, nil +} + +func TestScopedProviderPassesThroughAConfiguredEntity(t *testing.T) { + inner := &fixedProvider{intent: crydenai.QueryIntent{Entity: "sessions"}} + scoped := NewScopedProvider(inner, []string{"sessions", "audit_events"}) + + intent, err := scoped.ParseQueryIntent(context.Background(), "when did I last log in") + if err != nil { + t.Fatalf("ParseQueryIntent: %v", err) + } + if intent.Entity != "sessions" { + t.Errorf("entity = %q, want it passed through", intent.Entity) + } +} + +// The whole point of the type: an entity the deployment has not made +// available is refused, even though cryden's own allowlist permits it and +// widget.Ask would happily scope it. +func TestScopedProviderRefusesAnEntityOutsideTheConfiguredScope(t *testing.T) { + inner := &fixedProvider{intent: crydenai.QueryIntent{Entity: "audit_events"}} + scoped := NewScopedProvider(inner, []string{"sessions"}) + + _, err := scoped.ParseQueryIntent(context.Background(), "what has been recorded against me") + if !errors.Is(err, ErrEntityOutOfScope) { + t.Fatalf("error = %v, want ErrEntityOutOfScope", err) + } +} + +// A scope that was never configured answers nothing rather than +// everything. A settings form nobody filled in is not permission. +func TestScopedProviderWithNoEntitiesRefusesEverything(t *testing.T) { + for _, entities := range [][]string{nil, {}} { + scoped := NewScopedProvider(&fixedProvider{intent: crydenai.QueryIntent{Entity: "users"}}, entities) + + _, err := scoped.ParseQueryIntent(context.Background(), "who am I") + if !errors.Is(err, ErrEntityOutOfScope) { + t.Errorf("scope %v: error = %v, want ErrEntityOutOfScope", entities, err) + } + } +} + +// The inner provider's own failure is passed through unchanged: this +// wrapper narrows a scope, it does not reinterpret a model call that +// failed. +func TestScopedProviderPassesThroughTheInnerError(t *testing.T) { + sentinel := errors.New("the provider refused") + scoped := NewScopedProvider(&fixedProvider{err: sentinel}, []string{"sessions"}) + + _, err := scoped.ParseQueryIntent(context.Background(), "anything") + if !errors.Is(err, sentinel) { + t.Errorf("error = %v, want the inner provider's error", err) + } +} + +// The refusal has to name nothing. It reaches an end user through the +// widget, and an error that listed the deployment's configured entities +// would be describing the console's schema to whoever is typing +// questions at it. +func TestScopedProviderRefusalNamesNeitherEntityNorScope(t *testing.T) { + scoped := NewScopedProvider( + &fixedProvider{intent: crydenai.QueryIntent{Entity: "audit_events"}}, + []string{"sessions"}, + ) + + _, err := scoped.ParseQueryIntent(context.Background(), "anything") + if err == nil { + t.Fatal("nothing was refused") + } + message := err.Error() + for _, leak := range []string{"audit_events", "sessions"} { + if strings.Contains(message, leak) { + t.Errorf("error = %q, want it to name neither the refused entity nor the configured scope", message) + } + } +} diff --git a/config/config.go b/config/config.go index c63f35b..8b824a2 100644 --- a/config/config.go +++ b/config/config.go @@ -62,6 +62,25 @@ type Config struct { // JWT_SECRET. EncryptionKey string + // SettingsEncryptionKey seals the credentials this repo stores for + // the AI-assisted admin features — an LLM provider's API key, a + // read-only database's password — at rest, in the settings table. + // + // Deliberately NOT EncryptionKey, and this is the one place in this + // repo where a second key is spent rather than reused. EncryptionKey + // is cryden's: the engine derives from it whatever it needs to read + // TOTP secrets, and this repo never sees those bytes. This key is + // this repo's own, and the two have different lifetimes and different + // blast radii — rotating one must not silently make the other's rows + // unreadable. Every other purpose-keyed secret here follows the same + // rule (see CLOUD_LOG_HASH_KEY). + // + // Empty means the AI settings endpoints answer 404 not_configured + // rather than the server refusing to start, the same shape + // ENCRYPTION_KEY itself uses for the second factors: a deployment + // that has never opened that screen should still run. + SettingsEncryptionKey string + // TOTPIssuerName is what the user's authenticator app shows next to // the account. Cosmetic. Empty means cryden's own default ("Cryden"). TOTPIssuerName string @@ -116,6 +135,22 @@ type Config struct { RateLimitAttempts int RateLimitWindow time.Duration + // LockoutThreshold and LockoutDuration are the engine's account + // lockout bounds: after LockoutThreshold consecutive failed attempts the + // account is locked for LockoutDuration. Defaults are cryden's own (5 + // and 15 minutes), restated here for the same reason the rate-limit + // bounds are — and with a sharper edge, because the engine does not + // fill these in either way. A zero threshold locks an account on its + // very first failed password; a zero duration locks it until an instant + // already past, which is to say not at all. + // + // They are also what GET /v1/admin/config-tuning describes when it says + // a lockout setting is in force. Passed through to the engine rather + // than left implicit, so the report and the engine cannot disagree + // about what this deployment is actually running. + LockoutThreshold int + LockoutDuration time.Duration + // PasswordHasher selects which algorithm NEW password hashes are // written with — PasswordHasherBcrypt (the engine's default) or // PasswordHasherArgon2id. Switching is safe at any time and needs no @@ -233,6 +268,17 @@ type Config struct { // that retries forever is a load generator pointed at a third party — // and the row stays readable afterwards either way. WebhookMaxAttempts int + + // DigestInterval is how often the background job builds a digest and + // records it in digest_runs, which is what + // GET /v1/admin/digest/history reads back. Zero — the default — runs no + // job at all. + // + // Opt-in rather than "weekly by default", for the reason webhooks and + // cloud logging are: a deployment that has not asked for scheduled + // digests should not have a goroutine quietly accumulating rows. The + // on-demand GET /v1/admin/digest works either way, and writes nothing. + DigestInterval time.Duration } // PasswordHasher values. Bcrypt is the engine's own default and what an @@ -343,6 +389,12 @@ func Load() (Config, error) { } } + // This repo's own key, for the credentials it stores itself in the + // settings table. Read here rather than defaulted from EncryptionKey + // above, on purpose — see the field comment for why the two are + // separate secrets with separate lifetimes. + cfg.SettingsEncryptionKey = os.Getenv("SETTINGS_ENCRYPTION_KEY") + // Anomaly detection and credential-stuffing detection — one switch, // because they are one store. Both threshold sets begin as the // engine's defaults and every knob below only replaces the one it @@ -398,6 +450,22 @@ func Load() (Config, error) { return cfg, err } + // Account lockout. Defaulted rather than left at zero — see the field + // comments: a zero threshold would lock every account on its first + // failed password, which is the opposite of a default. A threshold + // below 1 is refused for the same reason rather than read as "off": + // cryden has no way to switch lockout off, so a 0 here can only be a + // typo, and honouring it would lock every account on one bad password. + if cfg.LockoutThreshold, err = envInt("LOCKOUT_THRESHOLD", 5); err != nil { + return cfg, err + } + if cfg.LockoutThreshold < 1 { + return cfg, fmt.Errorf("LOCKOUT_THRESHOLD must be at least 1, got %d — cryden has no way to switch account lockout off", cfg.LockoutThreshold) + } + if cfg.LockoutDuration, err = envMinutes("LOCKOUT_DURATION_MINUTES", 15*time.Minute); err != nil { + return cfg, err + } + // Password hashing. Bcrypt is the engine's default, so the only thing // this repo has to do for it is not pass a hasher — but the argon2id // parameters are assembled either way, because the hash-migration @@ -525,6 +593,19 @@ func Load() (Config, error) { } } + // Scheduled digests. Unset or 0 is off (see the field comment); a + // negative is refused rather than read as "off", because it can only + // be a typo and silently treating a typo as the default is how a + // setting an operator meant to change does nothing at all. + digestHours, err := envInt("DIGEST_INTERVAL_HOURS", 0) + if err != nil { + return cfg, err + } + if digestHours < 0 { + return cfg, fmt.Errorf("DIGEST_INTERVAL_HOURS cannot be negative — leave it unset to switch scheduled digests off, got %d", digestHours) + } + cfg.DigestInterval = time.Duration(digestHours) * time.Hour + return cfg, nil } diff --git a/config/config_test.go b/config/config_test.go index b1333b0..3570df8 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -44,6 +44,9 @@ var tieredEnvVars = []string{ "WEBHOOK_SECRET", "WEBHOOK_EVENTS", "WEBHOOK_MAX_ATTEMPTS", + "LOCKOUT_THRESHOLD", + "LOCKOUT_DURATION_MINUTES", + "DIGEST_INTERVAL_HOURS", } func loadForTest(t *testing.T, env map[string]string) (Config, error) { @@ -400,3 +403,84 @@ func TestTier3WebhookMaxAttemptsIsBounded(t *testing.T) { } } } + +// The Tier 4 defaults: cryden's own lockout numbers restated, because the +// engine takes them straight off its config with no defaulting of its own +// and both zero values are wrong in the same direction — a zero threshold +// locks an account on its first failed password, a zero duration locks it +// until an instant already past. +func TestTier4DefaultsComeFromTheEngine(t *testing.T) { + cfg, err := loadForTest(t, nil) + if err != nil { + t.Fatalf("Load() failed with only the required vars set: %v", err) + } + + if cfg.LockoutThreshold != 5 { + t.Errorf("LockoutThreshold = %d, want cryden's own default 5", cfg.LockoutThreshold) + } + if cfg.LockoutDuration != 15*time.Minute { + t.Errorf("LockoutDuration = %s, want cryden's own default 15m", cfg.LockoutDuration) + } + // Digests are opt-in: no schedule unless one was asked for, so an + // unconfigured deployment runs no goroutine and writes no rows. + if cfg.DigestInterval != 0 { + t.Errorf("DigestInterval = %s, want 0 (no schedule)", cfg.DigestInterval) + } +} + +func TestTier4EnvOverridesLeaveOtherKnobsDefaulted(t *testing.T) { + cfg, err := loadForTest(t, map[string]string{ + "LOCKOUT_THRESHOLD": "9", + "LOCKOUT_DURATION_MINUTES": "45", + "DIGEST_INTERVAL_HOURS": "168", + }) + if err != nil { + t.Fatalf("Load() failed: %v", err) + } + + if cfg.LockoutThreshold != 9 { + t.Errorf("LockoutThreshold = %d, want 9", cfg.LockoutThreshold) + } + if cfg.LockoutDuration != 45*time.Minute { + t.Errorf("LockoutDuration = %s, want 45m", cfg.LockoutDuration) + } + // A week in hours, which is the shape the env var is written in even + // though everything downstream holds a duration. + if cfg.DigestInterval != 168*time.Hour { + t.Errorf("DigestInterval = %s, want 168h", cfg.DigestInterval) + } + // Untouched knobs stay on their defaults. + if cfg.RateLimitAttempts != 10 || cfg.RateLimitWindow != time.Minute { + t.Errorf("rate limit = %d per %s, want the untouched default 10 per minute", cfg.RateLimitAttempts, cfg.RateLimitWindow) + } +} + +// A lockout threshold below 1 and a negative digest interval are both +// refused rather than read as "off": cryden has no way to switch account +// lockout off, and treating a typo as the default is how a setting an +// operator meant to change silently does nothing. +func TestTier4UnusableKnobValuesAreStartupErrors(t *testing.T) { + cases := []struct { + name string + env map[string]string + want string + }{ + {"threshold of zero", map[string]string{"LOCKOUT_THRESHOLD": "0"}, "LOCKOUT_THRESHOLD must be at least 1"}, + {"negative threshold", map[string]string{"LOCKOUT_THRESHOLD": "-1"}, "LOCKOUT_THRESHOLD must be at least 1"}, + {"non-numeric threshold", map[string]string{"LOCKOUT_THRESHOLD": "five"}, "LOCKOUT_THRESHOLD must be a number"}, + {"non-numeric lockout duration", map[string]string{"LOCKOUT_DURATION_MINUTES": "quarter of an hour"}, "LOCKOUT_DURATION_MINUTES must be a number of minutes"}, + {"negative digest interval", map[string]string{"DIGEST_INTERVAL_HOURS": "-1"}, "DIGEST_INTERVAL_HOURS cannot be negative"}, + {"non-numeric digest interval", map[string]string{"DIGEST_INTERVAL_HOURS": "weekly"}, "DIGEST_INTERVAL_HOURS must be a number"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + _, err := loadForTest(t, tc.env) + if err == nil { + t.Fatalf("%v was accepted, want an error", tc.env) + } + if !strings.Contains(err.Error(), tc.want) { + t.Errorf("error = %q, want it to contain %q", err, tc.want) + } + }) + } +} diff --git a/digest/memory.go b/digest/memory.go new file mode 100644 index 0000000..c0f7aba --- /dev/null +++ b/digest/memory.go @@ -0,0 +1,88 @@ +package digest + +import ( + "context" + "sort" + "sync" + "time" +) + +// MemoryStore is the in-process Store, for tests and for any embedding +// host that wants the digest history without a database behind it. +// +// It is a faithful double rather than a convenient one where the two +// implementations could quietly disagree: +// +// - List is ordered by GeneratedAt descending with ID as the tiebreak, +// which is the SQL's ORDER BY generated_at DESC, id DESC. A double +// that sorted only on the timestamp would pass every test while the +// real query returned a stable order the double did not have. +// - Insert applies the same zero-value stamping the Postgres store +// does, through the same resolveTimes, so a test asserting "the store +// filled in the clock" is asserting about real behaviour. +// +// What it does not reproduce is the database: there is no TIMESTAMPTZ +// round trip here, so a time that would not survive one is a difference +// this double cannot show. Postgres stores microseconds; Go's time.Time +// carries nanoseconds, and a monotonic reading is dropped on the way in. +// Nothing in flight depends on either — the window columns are compared +// against each other, never against a stored digest — but the gap is +// worth naming rather than assuming away. +type MemoryStore struct { + mu sync.Mutex + runs []Entry + next int64 + + // Clock stamps an entry that does not carry its own GeneratedAt, so a + // test can make a listing's ordering deterministic instead of hoping + // the wall clock separated two inserts. + Clock func() time.Time +} + +func NewMemoryStore() *MemoryStore { + return &MemoryStore{Clock: func() time.Time { return time.Now().UTC() }} +} + +var _ Store = (*MemoryStore)(nil) + +func (s *MemoryStore) Insert(_ context.Context, e Entry) (Entry, error) { + s.mu.Lock() + defer s.mu.Unlock() + + s.next++ + e.ID = s.next + e = resolveTimes(e, s.Clock()) + s.runs = append(s.runs, e) + return e, nil +} + +func (s *MemoryStore) List(_ context.Context, limit int) ([]Entry, error) { + s.mu.Lock() + defer s.mu.Unlock() + + out := make([]Entry, len(s.runs)) + copy(out, s.runs) + // Newest first, ties broken by id descending — the SQL's + // ORDER BY generated_at DESC, id DESC. + sort.SliceStable(out, func(i, j int) bool { + if !out[i].GeneratedAt.Equal(out[j].GeneratedAt) { + return out[i].GeneratedAt.After(out[j].GeneratedAt) + } + return out[i].ID > out[j].ID + }) + // Clamped here as well as by the handler, so a caller reaching the + // store directly gets the same bounded answer the endpoint gives. + limit = ClampLimit(limit) + if len(out) > limit { + out = out[:limit] + } + return out, nil +} + +// Count returns how many runs have been recorded. A test helper: no +// production caller needs a total, and no endpoint reports one. +func (s *MemoryStore) Count() int { + s.mu.Lock() + defer s.mu.Unlock() + return len(s.runs) +} diff --git a/digest/schedule.go b/digest/schedule.go new file mode 100644 index 0000000..473744a --- /dev/null +++ b/digest/schedule.go @@ -0,0 +1,107 @@ +package digest + +import ( + "context" + "log" + "time" +) + +// Builder produces one digest for the scheduler to record. It returns an +// Entry with ID unset — the store assigns one, and the store's own +// zero-value rule fills in whatever timestamp the builder leaves alone. +// +// It is a function rather than a store interface because the thing being +// wrapped is cryden's DigestSince, which takes the engine and returns a +// string. Threading a whole engine through this package to call it would +// mean this package importing the engine to describe a seam the caller +// can close in three lines. +type Builder func(ctx context.Context) (Entry, error) + +// Scheduler builds a digest on an interval and records each one. +// +// It is the only writer in this package, and it is a process component +// rather than anything a request can reach: no endpoint in this repo +// creates a digest run, so an operator cannot manufacture history +// through the API. That is the same shape the admin surface keeps +// everywhere else — see CLAUDE.md's hard rule. +type Scheduler struct { + Store Store + Build Builder + + // Interval is how long to wait between runs. Zero or negative means + // there is no schedule, and Run returns immediately without starting + // anything — main.go only constructs a Scheduler when + // DIGEST_INTERVAL_HOURS asked for one, so a zero here is a wiring + // mistake rather than a setting. + Interval time.Duration + + // Log receives one line per failed run. Optional; a nil Log discards + // them. + // + // Failures are logged and swallowed rather than returned: this runs in + // its own goroutine with nobody to hand an error to, and a scheduler + // that stopped on the first database blip would silently stop + // producing digests for the rest of the process's life — the exact + // failure a schedule exists to avoid. + Log *log.Logger +} + +// Run blocks until ctx is done, building and recording a digest once per +// Interval. +// +// The first run happens after a full Interval, not at startup. That is +// deliberate: the interval is the schedule, and a process that restarts +// more often than the interval elapses — a crashloop, a deploy pipeline, +// a developer's laptop — would otherwise manufacture one digest row per +// start. A history that grows with restarts rather than with time is not +// a history of anything. +// +// main.go hands this context.Background(), because this repo has no +// graceful shutdown yet — the same caveat, and the same reasoning, as the +// webhook worker's goroutine. Nothing here needs stopping today: an +// interrupted run loses at most one digest, and the next interval builds +// another. +func (s *Scheduler) Run(ctx context.Context) { + if s.Interval <= 0 || s.Store == nil || s.Build == nil { + return + } + + ticker := time.NewTicker(s.Interval) + defer ticker.Stop() + + for { + select { + case <-ctx.Done(): + return + case <-ticker.C: + s.runOnce(ctx) + } + } +} + +// runOnce builds one digest and records it, logging rather than returning +// any failure — see the Log field for why the loop must survive one. +func (s *Scheduler) runOnce(ctx context.Context) { + entry, err := s.Build(ctx) + if err != nil { + s.logf("digest: building the scheduled digest failed: %v", err) + return + } + + // The zero-value rule stamps GeneratedAt and WindowEnd if the builder + // left them; a builder that set neither still produces a readable row. + saved, err := s.Store.Insert(ctx, entry) + if err != nil { + s.logf("digest: recording the scheduled digest failed: %v", err) + return + } + s.logf("digest: recorded a scheduled digest covering %s to %s (run %d)", + saved.WindowStart.UTC().Format(time.RFC3339), saved.WindowEnd.UTC().Format(time.RFC3339), saved.ID) +} + +func (s *Scheduler) logf(format string, args ...any) { + if s.Log == nil { + return + } + s.Log.Printf(format, args...) +} diff --git a/digest/schedule_test.go b/digest/schedule_test.go new file mode 100644 index 0000000..de30138 --- /dev/null +++ b/digest/schedule_test.go @@ -0,0 +1,283 @@ +package digest + +import ( + "bytes" + "context" + "errors" + "log" + "strings" + "sync" + "testing" + "time" +) + +// syncBuffer collects log output written from a goroutine the test does not +// control. A bare bytes.Buffer would be a race the -race build is entitled +// to fail on, and the whole point of these tests is a loop running beside +// the assertion. +type syncBuffer struct { + mu sync.Mutex + buf bytes.Buffer +} + +func (b *syncBuffer) Write(p []byte) (int, error) { + b.mu.Lock() + defer b.mu.Unlock() + return b.buf.Write(p) +} + +func (b *syncBuffer) String() string { + b.mu.Lock() + defer b.mu.Unlock() + return b.buf.String() +} + +// builder is a Builder that counts its calls and hands back a fixed entry, +// so a test can ask whether it was called at all rather than infer it from +// a row. +type builder struct { + mu sync.Mutex + calls int + entry Entry + err error +} + +func (b *builder) build(context.Context) (Entry, error) { + b.mu.Lock() + defer b.mu.Unlock() + b.calls++ + return b.entry, b.err +} + +func (b *builder) callCount() int { + b.mu.Lock() + defer b.mu.Unlock() + return b.calls +} + +// A scheduler with nothing to schedule does nothing at all: no goroutine +// left ticking, no row written. This is the state main.go avoids by only +// constructing a Scheduler when DIGEST_INTERVAL_HOURS asked for one, so +// what is asserted here is that a mistake there stays inert. +func TestSchedulerDoesNothingWithoutASchedule(t *testing.T) { + for _, tc := range []struct { + name string + interval time.Duration + withStore bool + withBuild bool + }{ + {name: "no interval", withStore: true, withBuild: true}, + {name: "no interval and nothing else either"}, + {name: "no store", interval: time.Hour, withBuild: true}, + {name: "no builder", interval: time.Hour, withStore: true}, + } { + t.Run(tc.name, func(t *testing.T) { + store := NewMemoryStore() + build := &builder{entry: Entry{Text: "unreachable"}} + + s := &Scheduler{Interval: tc.interval} + if tc.withStore { + s.Store = store + } + if tc.withBuild { + s.Build = build.build + } + + // Run is expected back promptly rather than at the end of an + // interval: an unconfigured scheduler must not hold a goroutine + // open for an hour first. + done := make(chan struct{}) + go func() { + defer close(done) + s.Run(context.Background()) + }() + select { + case <-done: + case <-time.After(2 * time.Second): + t.Fatal("Run is still going with nothing to run") + } + + if n := store.Count(); n != 0 { + t.Errorf("recorded %d runs, want 0", n) + } + if n := build.callCount(); n != 0 { + t.Errorf("the builder was called %d times, want 0", n) + } + }) + } +} + +// The first run is after a full interval, not at startup. Asserted against +// an hour-long interval, so this cannot pass by being slow: if the run +// happened at startup it would have happened within microseconds of Run +// being called, and the check below waits a hundred milliseconds. +// +// It is the property that keeps a crashlooping process from manufacturing +// one row per restart, which is the failure mode a history table is worst +// at showing — the rows look like a busy week. +func TestSchedulerWaitsAFullIntervalBeforeTheFirstRun(t *testing.T) { + store := NewMemoryStore() + build := &builder{entry: Entry{Text: "the first run"}} + s := &Scheduler{Store: store, Build: build.build, Interval: time.Hour} + + ctx, cancel := context.WithCancel(context.Background()) + done := make(chan struct{}) + go func() { + defer close(done) + s.Run(ctx) + }() + + time.Sleep(100 * time.Millisecond) + if n := build.callCount(); n != 0 { + t.Errorf("the builder ran %d times before the first interval elapsed, want 0", n) + } + if n := store.Count(); n != 0 { + t.Errorf("recorded %d runs before the first interval elapsed, want 0", n) + } + + // And it stops when the context is cancelled rather than at the next + // tick, which is what a caller with a shutdown path would need. + cancel() + select { + case <-done: + case <-time.After(2 * time.Second): + t.Fatal("Run did not return after its context was cancelled") + } +} + +// The positive half: one run per interval, each stored with the window the +// builder computed. The interval is deliberately tiny so the test is +// seconds-cheap; what is being asserted is that the loop records, not how +// often it does. +func TestSchedulerRecordsOneRunPerInterval(t *testing.T) { + store := NewMemoryStore() + since := time.Date(2026, 8, 1, 0, 0, 0, 0, time.UTC) + build := &builder{entry: Entry{WindowStart: since, Text: "the weekly report"}} + s := &Scheduler{Store: store, Build: build.build, Interval: 5 * time.Millisecond} + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + go s.Run(ctx) + + waitForRuns(t, store, 2) + + rows, err := store.List(context.Background(), 10) + if err != nil { + t.Fatalf("List: %v", err) + } + if rows[0].Text != "the weekly report" { + t.Errorf("text = %q, want the builder's report stored verbatim", rows[0].Text) + } + if !rows[0].WindowStart.Equal(since) { + t.Errorf("window_start = %v, want the window the builder computed %v", rows[0].WindowStart, since) + } + if rows[0].ID == 0 { + t.Error("a recorded run has no id") + } + if rows[0].GeneratedAt.IsZero() || rows[0].WindowEnd.IsZero() { + t.Errorf("run %+v was stored without its times filled in", rows[0]) + } +} + +// A failed build is logged and the loop keeps going. A scheduler that +// stopped at the first database blip would silently stop producing +// digests for the rest of the process's life — the exact failure a +// schedule exists to avoid — and one that retried instantly would spin. +func TestSchedulerSurvivesAFailedBuild(t *testing.T) { + store := NewMemoryStore() + logs := &syncBuffer{} + + var mu sync.Mutex + attempts := 0 + build := func(context.Context) (Entry, error) { + mu.Lock() + defer mu.Unlock() + attempts++ + if attempts == 1 { + return Entry{}, errors.New("the audit table is unreachable") + } + return Entry{Text: "the second attempt"}, nil + } + + s := &Scheduler{Store: store, Build: build, Interval: 5 * time.Millisecond, Log: log.New(logs, "", 0)} + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + go s.Run(ctx) + + waitForRuns(t, store, 1) + + if got := logs.String(); !strings.Contains(got, "the audit table is unreachable") { + t.Errorf("log = %q, want the build failure recorded", got) + } + rows, err := store.List(context.Background(), 10) + if err != nil { + t.Fatalf("List: %v", err) + } + if rows[0].Text != "the second attempt" { + t.Errorf("text = %q, want the run that succeeded after the failure", rows[0].Text) + } +} + +// A builder that fails every time writes nothing rather than a row of +// empty text: an entry with no report in it is worse than no entry, because +// a history listing cannot tell it apart from a quiet week. +func TestSchedulerRecordsNothingWhenTheStoreRejectsTheRun(t *testing.T) { + store := &refusingStore{} + s := &Scheduler{Store: store, Build: func(context.Context) (Entry, error) { + return Entry{Text: "never stored"}, nil + }, Interval: 5 * time.Millisecond, Log: log.New(&syncBuffer{}, "", 0)} + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + go s.Run(ctx) + + deadline := time.Now().Add(2 * time.Second) + for store.insertAttempts() < 3 && time.Now().Before(deadline) { + time.Sleep(time.Millisecond) + } + if n := store.insertAttempts(); n < 3 { + t.Fatalf("the scheduler made %d insert attempts in two seconds, want the loop to keep trying", n) + } + if n := store.count(); n != 0 { + t.Errorf("%d runs were recorded by a store that refused every insert", n) + } +} + +// waitForRuns blocks until the store holds at least n runs, failing the +// test rather than hanging if the loop never gets there. +func waitForRuns(t *testing.T, store *MemoryStore, n int) { + t.Helper() + deadline := time.Now().Add(5 * time.Second) + for store.Count() < n { + if time.Now().After(deadline) { + t.Fatalf("the scheduler recorded %d runs in five seconds, want %d", store.Count(), n) + } + time.Sleep(time.Millisecond) + } +} + +// refusingStore counts insert attempts and rejects all of them, which is +// what a database that is down looks like from the scheduler's side. +type refusingStore struct { + mu sync.Mutex + attempts int +} + +func (s *refusingStore) Insert(context.Context, Entry) (Entry, error) { + s.mu.Lock() + defer s.mu.Unlock() + s.attempts++ + return Entry{}, errors.New("the database is unreachable") +} + +func (s *refusingStore) List(context.Context, int) ([]Entry, error) { return nil, nil } + +func (s *refusingStore) insertAttempts() int { + s.mu.Lock() + defer s.mu.Unlock() + return s.attempts +} + +func (s *refusingStore) count() int { return 0 } + +var _ Store = (*refusingStore)(nil) diff --git a/digest/store.go b/digest/store.go new file mode 100644 index 0000000..7185621 --- /dev/null +++ b/digest/store.go @@ -0,0 +1,214 @@ +// Package digest keeps the history of the reports this deployment's +// weekly digest produced, and runs the schedule that produces them. +// +// # Why this is here and not in the engine +// +// cryden's WeeklyDigest/DigestSince build a report out of the audit table +// and hand back a string. There is no scheduler in the engine, no run +// record, and nothing that remembers a digest was ever generated — that +// is deliberate on cryden's side, because "when should this fire" and +// "where should it be kept" are deployment questions, not authentication +// ones. So the history table, the background job that fills it, and the +// endpoint that reads it back are all this repo's own. +// +// # Read-only, like everything else on the admin surface +// +// GET /v1/admin/digest and GET /v1/admin/digest/history only ever read, +// and neither of them writes a row: the on-demand endpoint deliberately +// does NOT record what it built, so asking for a digest twice does not +// fabricate two entries in a history an operator reads as a record of +// what was scheduled. Only Scheduler writes, and it is a process +// component rather than a request handler — see CLAUDE.md's hard rule. +// +// # The row is the report, not a recipe for one +// +// Text is stored rather than the counts behind it. A digest covers a +// window that has ended, so re-running its query later would not +// reproduce it: "the last seven days" is anchored to when the digest was +// built. Storing what was actually reported is what makes reading a past +// digest the same experience as reading a fresh one. +package digest + +import ( + "context" + "database/sql" + "errors" + "fmt" + "time" +) + +// Entry is one recorded digest. +type Entry struct { + // ID is assigned by the store and is zero on the value handed to + // Insert. + ID int64 + + // WindowStart and WindowEnd bound the report: everything counted + // happened at or after WindowStart, and WindowEnd is the instant the + // digest was built. Carried as fields rather than parsed back out of + // Text, so a listing can sort and describe runs without reading + // English out of a report. + WindowStart time.Time + WindowEnd time.Time + + // GeneratedAt is when the row was written. Distinct from WindowEnd on + // purpose: a run replayed after an outage covers a window that closed + // before the digest was made. + GeneratedAt time.Time + + // Text is the rendered report, exactly as the engine returned it. + // Stored verbatim and never re-rendered — this repo does not format + // cryden's reports, and a copy reformatted here would be a second + // implementation of a report the engine already owns. + Text string +} + +// Store is the persistence seam. Two implementations: PostgresStore and +// MemoryStore, the in-memory double the tests use — the same split every +// repo-owned store in this repo follows. +type Store interface { + // Insert records one run and returns it with its assigned ID. + // + // Unlike webhook deliveries there is no dedupe and no "already + // recorded" case: two runs of the same window are two things that + // happened, and the second one is exactly what an operator wants to + // see when they suspect the schedule fired twice. + Insert(ctx context.Context, e Entry) (Entry, error) + + // List returns runs newest first, at most limit of them. Ordered by + // GeneratedAt with ID as the tiebreak, so the order is total and two + // runs sharing a timestamp do not swap places between requests. + List(ctx context.Context, limit int) ([]Entry, error) +} + +// ErrNotFound is returned when a run is asked for by an ID that is not +// there. Nothing in this repo reads by ID today — the history is listed, +// never fetched — so this exists for a caller that grows one, and for the +// in-memory double to mean the same thing the Postgres store means. +var ErrNotFound = errors.New("digest run not found") + +// DefaultHistoryLimit bounds a history listing that does not ask for a +// size, and MaxHistoryLimit is the ceiling a request may ask for. A +// digest is at most one row per interval — 52 a year on the weekly +// default — so this is a bound against a caller looping with a large +// limit, not against ordinary volume. +const ( + DefaultHistoryLimit = 20 + MaxHistoryLimit = 200 +) + +// Columns is one const so the scan and the query cannot drift apart — the +// same reason shiplog's logColumns is one. +const runColumns = `id, window_start, window_end, generated_at, digest_text` + +// resolveTimes applies the zero-value rule both stores share: an entry +// that does not carry its own timestamps is stamped by whichever clock +// the store owns, so Postgres stamps with the database host's clock and +// the in-memory double stamps with the test's. +// +// WindowEnd falls back to GeneratedAt rather than to the fallback clock +// directly: the two are the same instant for every run this package +// writes, and deriving one from the other keeps them equal even for a +// caller that supplied only a GeneratedAt. +func resolveTimes(e Entry, fallback time.Time) Entry { + if e.GeneratedAt.IsZero() { + e.GeneratedAt = fallback + } + if e.WindowEnd.IsZero() { + e.WindowEnd = e.GeneratedAt + } + return e +} + +// PostgresStore is the durable Store. +type PostgresStore struct { + db *sql.DB +} + +func NewStore(db *sql.DB) *PostgresStore { + return &PostgresStore{db: db} +} + +var _ Store = (*PostgresStore)(nil) + +func (s *PostgresStore) Insert(ctx context.Context, e Entry) (Entry, error) { + e = resolveTimes(e, time.Now().UTC()) + + err := s.db.QueryRowContext(ctx, + `INSERT INTO digest_runs (window_start, window_end, generated_at, digest_text) + VALUES ($1, $2, $3, $4) + RETURNING id`, + e.WindowStart, e.WindowEnd, e.GeneratedAt, e.Text, + ).Scan(&e.ID) + if err != nil { + return Entry{}, err + } + return e, nil +} + +func (s *PostgresStore) List(ctx context.Context, limit int) ([]Entry, error) { + // Clamped inside the store rather than only at the handler, because + // `LIMIT $1` with a zero returns nothing and with a negative is a + // Postgres error — so an unclamped store answers a caller that + // reached it directly with either a broken listing or a driver fault, + // while the in-memory double would have answered sensibly. Applying + // the same rule in both is what keeps them the same store. + limit = ClampLimit(limit) + + rows, err := s.db.QueryContext(ctx, + `SELECT `+runColumns+` + FROM digest_runs + ORDER BY generated_at DESC, id DESC + LIMIT $1`, + limit, + ) + if err != nil { + return nil, err + } + defer rows.Close() + + runs := make([]Entry, 0, limit) + for rows.Next() { + e, err := scanRun(rows) + if err != nil { + return nil, err + } + runs = append(runs, e) + } + return runs, rows.Err() +} + +// scanRun reads one row. Kept as a function so the column list and the +// scan order are read together. +func scanRun(rows *sql.Rows) (Entry, error) { + var e Entry + if err := rows.Scan(&e.ID, &e.WindowStart, &e.WindowEnd, &e.GeneratedAt, &e.Text); err != nil { + return Entry{}, fmt.Errorf("scanning a digest run: %w", err) + } + return e, nil +} + +// ClampLimit narrows a requested history limit into range. Shared by both +// implementations, so the bound is applied in one place rather than +// restated at each of the two. +// +// A non-positive limit means "the default" rather than "none", because +// `LIMIT 0` is a listing that looks broken while a negative limit is a +// driver error — so without this, a caller reaching the store directly +// would get an answer the endpoint would never give. +// +// The endpoint itself does not rely on the clamp: it bounds the parameter +// and answers 400 outside 1..MaxHistoryLimit, so a console asking for +// limit=0 is told its request was wrong rather than handed a default it +// did not ask for (see httpapi's queryInt and the same rule in the logging +// endpoint). This is what a direct caller gets. +func ClampLimit(limit int) int { + switch { + case limit <= 0: + return DefaultHistoryLimit + case limit > MaxHistoryLimit: + return MaxHistoryLimit + default: + return limit + } +} diff --git a/digest/store_test.go b/digest/store_test.go new file mode 100644 index 0000000..64d2eb8 --- /dev/null +++ b/digest/store_test.go @@ -0,0 +1,248 @@ +package digest + +import ( + "context" + "fmt" + "sync" + "testing" + "time" +) + +// testClock is a hand-wound clock, so an ordering assertion is about the +// store's sort rather than about how far apart two time.Now() calls +// happened to land. The same idea as webhook's, in this package's own +// terms because a double is only worth having if it is the double the +// store under test actually reads. +type testClock struct { + mu sync.Mutex + at time.Time +} + +func newTestClock() *testClock { + // A fixed instant, not now: nothing here should depend on when the + // suite runs. + return &testClock{at: time.Date(2026, 8, 1, 12, 0, 0, 0, time.UTC)} +} + +func (c *testClock) now() time.Time { + c.mu.Lock() + defer c.mu.Unlock() + return c.at +} + +func (c *testClock) advance(d time.Duration) { + c.mu.Lock() + defer c.mu.Unlock() + c.at = c.at.Add(d) +} + +// newTestStore is a MemoryStore on a clock the test controls. +func newTestStore() (*MemoryStore, *testClock) { + clock := newTestClock() + s := NewMemoryStore() + s.Clock = clock.now + return s, clock +} + +func insert(t *testing.T, s Store, e Entry) Entry { + t.Helper() + saved, err := s.Insert(context.Background(), e) + if err != nil { + t.Fatalf("Insert: %v", err) + } + return saved +} + +// The zero-value rule, in all three of its cases. The third is the one +// worth stating: WindowEnd falls back to the entry's own GeneratedAt +// rather than to the clock, so a caller that supplied a GeneratedAt gets +// a WindowEnd equal to it instead of one a few microseconds later. +func TestMemoryStoreStampsOnlyTheTimesItIsNotGiven(t *testing.T) { + ctx := context.Background() + s, clock := newTestStore() + + bare := insert(t, s, Entry{Text: "nothing given"}) + if bare.ID != 1 { + t.Errorf("id = %d on the first insert, want 1", bare.ID) + } + if !bare.GeneratedAt.Equal(clock.now()) { + t.Errorf("generated_at = %v, want the store's clock %v", bare.GeneratedAt, clock.now()) + } + if !bare.WindowEnd.Equal(bare.GeneratedAt) { + t.Errorf("window_end = %v with nothing given, want the generated_at %v", bare.WindowEnd, bare.GeneratedAt) + } + + clock.advance(time.Hour) + start := clock.now().Add(-7 * 24 * time.Hour) + end := clock.now() + partial := insert(t, s, Entry{WindowStart: start, WindowEnd: end, Text: "window given"}) + if !partial.WindowStart.Equal(start) || !partial.WindowEnd.Equal(end) { + t.Errorf("window = %v..%v, want the one supplied %v..%v", partial.WindowStart, partial.WindowEnd, start, end) + } + if !partial.GeneratedAt.Equal(clock.now()) { + t.Errorf("generated_at = %v, want the store's clock for a run that did not state one", partial.GeneratedAt) + } + + // A GeneratedAt in the past with no WindowEnd: the fallback is the + // entry's own timestamp, not the clock. + past := clock.now().Add(-30 * time.Hour) + derived := insert(t, s, Entry{GeneratedAt: past, Text: "generated given"}) + if !derived.WindowEnd.Equal(past) { + t.Errorf("window_end = %v, want the supplied generated_at %v rather than the clock", derived.WindowEnd, past) + } + + // And what was written is what List reads back: the stamping is the + // store's, not a field the caller's copy got and the row did not. + rows, err := s.List(ctx, 10) + if err != nil { + t.Fatalf("List: %v", err) + } + if len(rows) != 3 { + t.Fatalf("listed %d runs, want 3", len(rows)) + } + for _, row := range rows { + if row.GeneratedAt.IsZero() || row.WindowEnd.IsZero() { + t.Errorf("run %d read back with a zero timestamp: %+v", row.ID, row) + } + } +} + +// Newest first, with ID as the tiebreak — the SQL's ORDER BY +// generated_at DESC, id DESC. A double that sorted on the timestamp alone +// would pass every other test in this file and still hand two runs +// sharing an instant back in an order the real query does not. +func TestMemoryStoreListsNewestFirstBreakingTiesByID(t *testing.T) { + s, clock := newTestStore() + + for _, text := range []string{"first", "second", "third"} { + insert(t, s, Entry{Text: text}) + clock.advance(time.Hour) + } + // Two runs in the same instant, which is what a replayed schedule or a + // clock with second-granularity storage produces. + same := clock.now() + older := insert(t, s, Entry{GeneratedAt: same, Text: "same instant, lower id"}) + newer := insert(t, s, Entry{GeneratedAt: same, Text: "same instant, higher id"}) + + rows, err := s.List(context.Background(), 10) + if err != nil { + t.Fatalf("List: %v", err) + } + if len(rows) != 5 { + t.Fatalf("listed %d runs, want 5", len(rows)) + } + if rows[0].ID != newer.ID || rows[1].ID != older.ID { + t.Errorf("the tied pair came back as %d then %d, want %d then %d", + rows[0].ID, rows[1].ID, newer.ID, older.ID) + } + if rows[2].Text != "third" { + t.Errorf("third row = %q, want the next-newest timestamp", rows[2].Text) + } + if rows[4].Text != "first" { + t.Errorf("last row = %q, want the oldest", rows[4].Text) + } +} + +// The listing is bounded, and a non-positive limit means the default +// rather than nothing. Both implementations clamp through ClampLimit; the +// Postgres one is not exercised here — this environment has no Postgres — +// so what is asserted is the rule they share. +func TestMemoryStoreBoundsTheListing(t *testing.T) { + s, _ := newTestStore() + for i := 0; i < DefaultHistoryLimit+5; i++ { + insert(t, s, Entry{Text: fmt.Sprintf("run %d", i)}) + } + + rows, err := s.List(context.Background(), 3) + if err != nil { + t.Fatalf("List: %v", err) + } + if len(rows) != 3 { + t.Errorf("listed %d runs with limit 3, want 3", len(rows)) + } + if rows[0].Text != fmt.Sprintf("run %d", DefaultHistoryLimit+4) { + t.Errorf("first row = %q, want the newest of the bounded set", rows[0].Text) + } + + for _, limit := range []int{0, -1} { + rows, err := s.List(context.Background(), limit) + if err != nil { + t.Fatalf("List(%d): %v", limit, err) + } + if len(rows) != DefaultHistoryLimit { + t.Errorf("listed %d runs with limit %d, want the default %d — a non-positive limit means the default, not none", + len(rows), limit, DefaultHistoryLimit) + } + } +} + +func TestClampLimit(t *testing.T) { + for _, tc := range []struct { + in, want int + }{ + {5, 5}, + {DefaultHistoryLimit, DefaultHistoryLimit}, + {MaxHistoryLimit, MaxHistoryLimit}, + {MaxHistoryLimit + 1, MaxHistoryLimit}, + {0, DefaultHistoryLimit}, + {-7, DefaultHistoryLimit}, + } { + if got := ClampLimit(tc.in); got != tc.want { + t.Errorf("ClampLimit(%d) = %d, want %d", tc.in, got, tc.want) + } + } +} + +// A returned listing is a copy, so a caller cannot edit the history by +// editing what it was handed — the thing a scanned row cannot do to a +// database either. +func TestMemoryStoreHandsOutCopies(t *testing.T) { + s, _ := newTestStore() + insert(t, s, Entry{Text: "the real text"}) + + rows, _ := s.List(context.Background(), 10) + rows[0].Text = "edited by a caller" + rows[0].ID = 999 + + again, _ := s.List(context.Background(), 10) + if again[0].Text != "the real text" || again[0].ID != 1 { + t.Errorf("stored run is %+v after a caller edited its copy", again[0]) + } +} + +// The store is written by the scheduler goroutine while HTTP handlers read +// it, which is the one piece of concurrency this feature actually has. +// Under -race this is what proves the mutex covers both paths. +func TestMemoryStoreIsSafeUnderConcurrentUse(t *testing.T) { + s := NewMemoryStore() + const writers, each = 8, 25 + + var wg sync.WaitGroup + for w := 0; w < writers; w++ { + wg.Add(1) + go func(w int) { + defer wg.Done() + for i := 0; i < each; i++ { + if _, err := s.Insert(context.Background(), Entry{Text: fmt.Sprintf("w%d-%d", w, i)}); err != nil { + t.Errorf("Insert: %v", err) + return + } + } + }(w) + } + wg.Add(1) + go func() { + defer wg.Done() + for i := 0; i < each; i++ { + if _, err := s.List(context.Background(), 5); err != nil { + t.Errorf("List: %v", err) + return + } + } + }() + wg.Wait() + + if got := s.Count(); got != writers*each { + t.Errorf("recorded %d runs, want %d", got, writers*each) + } +} diff --git a/docs/development/CURRENT-STATE.md b/docs/development/CURRENT-STATE.md index 18acd56..04efa0b 100644 --- a/docs/development/CURRENT-STATE.md +++ b/docs/development/CURRENT-STATE.md @@ -15,9 +15,18 @@ has its own `httpapi/apple.go` — see `NEXT.md` Tier 1). Tier 2 added one admin endpoint on top of those, the first in this repo — see below. Tier 3 added three more admin endpoints and this repo's first three tables of its own, plus the config that lights up Argon2id, -cloud logging and email templates — see below. Every admin endpoint in -this repo is either read-only or an explicit operator action on a named -key; nothing on that surface applies a suggestion by itself. +cloud logging and email templates — see below. Tier 4 added six more +admin endpoints and two more tables of its own: Stage 1 is the weekly +digest and its recorded history, the support-ticket login diagnosis and +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 +`/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. Tier 1 also added the second-factor surface: TOTP enroll/confirm/ disable, passkey registration/list/delete, magic-link request/complete, @@ -38,7 +47,7 @@ still in-memory and single-process either way, the same caveat cryden's own default limiter carries. Response envelope, error codes, and the migration-copying convention -are all established — see `README.md` and `CODEX.md`. +are all established — see `README.md` and `CLAUDE.md`. ## Tier 0 — bump the engine to v2.5.0: DONE @@ -64,7 +73,7 @@ anything else in this repo. ## Tier 0.5 — admin/operator authorization foundation: DONE Console operator status is a concept this repo owns entirely, not -cryden — see `CODEX.md`'s ownership section for why. +cryden — see `CLAUDE.md`'s ownership section for why. - `migrations/003_operators.up.sql` / `.down.sql` — a new `operators` table, `user_id` (references cryden's own `users.id`), `role` (plain @@ -98,7 +107,7 @@ whichever Tier 4/5 endpoint lands first. ## Tier 1 — auth methods: DONE -Built on `feat/tier1-auth-methods` (its own branch, per `CODEX.md`'s +Built on `feat/tier1-auth-methods` (its own branch, per `CLAUDE.md`'s one-branch-per-tier rule), in this order: - `migrations/004`-`008` — cryden's `0003`-`0007` copied in, renumbered @@ -305,10 +314,158 @@ in-memory double, not against Postgres `FOR UPDATE SKIP LOCKED`, and that double cannot reproduce two workers racing. `PROGRESS.md` says all of this plainly. -## Tier 4 and 5 +## Tier 4 — AI-assisted admin endpoints: DONE + +Built in two stages on `feat/tier4-ai-admin-endpoints`, for the same +reason Tier 3 was: the three read-only reports below had their decisions +already made in `NEXT.md`, while Stage 2 needed two decisions that are +not a build session's to make. Those two were resolved by following +`NEXT.md`'s own instruction to make the reasonable call and record it — +see Stage 2 below. `go build`, `go vet`, `gofmt -l` and `go test ./...` +are clean, `httpapi` and the two new packages are also green under +`-race`, and `PROGRESS.md` records what that does and does not cover, +which is a lot. + +### Stage 1 — the three read-only reports + +Everything here is `RequireAdmin`, read-only, and buildable on the +engine alone — no LLM, no second database connection, no outbound call: + +- **`GET /v1/admin/digest`** and **`GET /v1/admin/digest/history`**. + The engine's `cryden.DigestSince` renders a report over a window on + demand; the history is this repo's own (`digest/`, + `migrations/012_digest_runs`), written only by the scheduled job. The + on-demand endpoint **records nothing**, so an operator hitting it + twenty times does not fill the history with twenty near-identical + reports. The engine has no scheduling concept at all, so the job, the + table and the history endpoint are all entirely this repo's. The first + run lands one full interval after startup rather than at boot, since a + process that restarts more often than the interval elapses would + otherwise write a row per restart. Unset `DIGEST_INTERVAL_HOURS` (or + `0`) means no schedule, no goroutine and a `404 not_configured` + history, while the on-demand endpoint keeps working. +- **`GET /v1/admin/support/diagnose?email=...`** → + `cryden.DiagnoseLoginIssue`. An unknown address is an **answer** + (`found: false`), not a `404`: "we have never seen this address" is + what a support ticket needs to be told. It describes a locked account; + it cannot unlock one. +- **`GET /v1/admin/config-tuning`** → `admin.BuildTuningReport` called + **directly**, not `cryden.ConfigTuningReport`. The structured + `TuningSuggestion{Area, Finding, Suggestion}` list is the point — a + console renders one card per suggestion, and a pre-rendered text blob + cannot be turned back into cards. The raw audit `counts` are returned + alongside, so the evidence is visible rather than a sentence asking to + be trusted. `window_days` defaults to cryden's own 30 days (wider than + the digest's week on purpose — a config knob should be judged against + a month of traffic) and is bounded rather than clamped. The route + accepts **GET and nothing else**, which is the HTTP-level half of the + pre-fill-never-auto-apply decision: there is no POST that takes a + suggestion, and applying one means pre-filling a settings field a + human saves through the ordinary settings path. + +**One behaviour change came out of this tier, and it was not the point +of it.** The tuning report is asked to judge the audit history against +"the settings in force", and building it surfaced that this repo had +never passed `LockoutThreshold`/`LockoutDuration` to the engine — so +every deployment so far ran with both at Go's zero value, and cryden +defaults neither. A zero threshold locks an account on its first failed +password; a zero duration locks it until an instant already past, which +is to say not at all. `config` now owns both knobs (cryden's own 5 and +15 minutes by default), `main.go` passes them, and a threshold below 1 +is a startup error rather than being read as "off", because cryden has +no way to switch lockout off. It is a real change to what every existing +deployment does on its next restart, so it is called out in `README.md`, +`.env.example` and its commit message rather than buried. + +The digest's new table (`012`) has **never been applied to a real +database**, the same as `009`–`011` — there is no Postgres in this +sandbox — and the digest schedule is a goroutine on +`context.Background()`, because this repo still has no graceful +shutdown. `PROGRESS.md` says both plainly. + +### Stage 2 — the providers and the widget config + +This is the half of the tier that needed an LLM, a second database +connection and an outbound call, so it is the half where this repo +stopped being purely a wrapper. Three settings endpoints, all +`RequireAdmin`, all in `httpapi/settings_handlers.go`, backed by +`settings/` and `migrations/013_settings`: + +- **`GET`/`PUT`/`DELETE /v1/admin/settings/llm-provider`** stores which + model and key back `ai.LLMProvider`. `DELETE` was added alongside the + specced pair: a settings screen with no way to clear a credential is + a screen an operator cannot leave. +- **`GET`/`PUT`/`DELETE /v1/admin/settings/database-provider`** stores + the connection `ai.QueryableStore` runs against. +- **`GET`/`PUT`/`DELETE /v1/admin/settings/ask-ai-widget`** stores the + widget's enabled flag, allowed origins, entity scope and copy. + +Both credentials are sealed with **AES-256-GCM before they reach the +table**, keyed from a new `SETTINGS_ENCRYPTION_KEY`. That key is +deliberately *not* cryden's `ENCRYPTION_KEY`: the two seal different +things with different lifetimes and blast radii, and this repo already +sets the precedent with `CLOUD_LOG_HASH_KEY`. An unset key is not a +startup failure — the three endpoints answer `404 not_configured`, like +every other optional feature here. The encryption itself is cryden's +`security.NewAESGCMEncryptor` rather than a second implementation of the +same primitive; see `settings/secrets.go`. + +Three things in this stage are worth reading before touching them: + +- **`PUT /database-provider` proves the role cannot write, then stores.** + Order is the whole design: validate the shape, connect with the + supplied credentials and attempt a write, and only store once the + server refuses. The probe targets `pg_temp`, so a failed probe leaves + nothing behind, and the pool is pinned to one connection so the + `CREATE` and the `INSERT` share the session owning that temp table. + Three outcomes are distinguished — refused is a pass, succeeded is + `400 database_role_not_read_only`, anything else is + `400 database_role_unverified` and **is not a pass**. Only SQLSTATE + `42501` counts as a refusal, matched by code rather than message. +- **`aiprovider.ScopedProvider` gives the widget's `entities` setting + teeth.** cryden's `widget.Ask` force-scopes every parsed intent to the + calling end user's own rows, overwriting whatever identity filter the + model produced rather than validating it — no oracle — but it does so + over all of `ai.AllowedEntities`. Narrowing that is a host decision, so + this repo refuses an out-of-scope entity in front of the provider. +- **`settings.AskAIWidgetConfig` is not a credential**, and that is why + it has no `Redacted` counterpart while the other two do. All three are + stored through the same `Secrets` wrapper anyway — one storage path + with one rule about what reaches the table is worth more than saving a + decryption. + +`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. + +What Stage 2 does **not** have evidence for, and `PROGRESS.md` says in +full: `CheckReadOnly` has never run against a real Postgres (the tested +branch is the *unverifiable* one, not the pass), the Anthropic provider +has never called Anthropic (it is tested against a local fake in the +Messages API's wire shape), and `013_settings` has never been applied to +a database. + +**The read-only rule now has a named exception, and it is this one.** +`/v1/admin/settings/*` is the admin surface's first write. The reading +is that `CLAUDE.md`'s rule covers the AI *tools* — which cryden builds +through interfaces carrying no way to act — rather than every route +under `/v1/admin`, and that a settings save is exactly what `NEXT.md`'s +pre-fill-never-auto-apply decision names as the human half. No +AI-assisted handler holds a reference to these routes, and none accepts +a suggestion as input. The alternative readings (store the key in +cryden, or environment-only) are worse and one of them is explicitly +ruled out by `NEXT.md`, which says this repo owns that config storage. + +## Tier 5 Not started. See `NEXT.md` for the full, ordered, specced-in-detail -queue. Tier 4 is all behind `RequireAdmin` and stays read-only by -construction, with the decision already made that an AI suggestion -**pre-fills** a settings form and never auto-applies. +queue — the users admin surface, which has no engine gap and is just +missing endpoints, plus the widget's own serving endpoint, which is what +the Stage 2 config above is waiting for. diff --git a/docs/development/NEXT.md b/docs/development/NEXT.md index 5a28e13..eed060c 100644 --- a/docs/development/NEXT.md +++ b/docs/development/NEXT.md @@ -1,12 +1,12 @@ # api — next up Ordered queue. Take the first unfinished item, build it completely, -verify it (see `CODEX.md`), update the three docs +verify it (see `CLAUDE.md`), update the three docs (`CURRENT-STATE.md`/`NEXT.md`/`PROGRESS.md`), then stop for review before starting the next tier. Specs below are deliberately detailed so you don't need to ask anything mid-build — where something is genuinely unspecified, make the most reasonable call consistent with -`CODEX.md`'s ownership rules and note the assumption in `PROGRESS.md`. +`CLAUDE.md`'s ownership rules and note the assumption in `PROGRESS.md`. Tier 0 and Tier 0.5 are done — see `CURRENT-STATE.md`. Tier 1 is done — see the status note under Tier 1 and `PROGRESS.md`'s @@ -16,6 +16,10 @@ 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. --- @@ -29,7 +33,7 @@ the status note under Tier 3 and `PROGRESS.md`'s 2026-09-15 entries. > What is still owed: a first DB-backed smoke-test run (no Postgres in > this sandbox), a live Apple round trip (no Apple credentials here), and > the WebAuthn ceremonies, which need a real browser authenticator. -> `PROGRESS.md` says all of that plainly, per `CODEX.md`'s verification +> `PROGRESS.md` says all of that plainly, per `CLAUDE.md`'s verification > rule, rather than counting green unit tests as end-to-end coverage. Each of these mirrors an existing engine feature that already has a @@ -284,8 +288,83 @@ Two details were decided rather than assumed, and are recorded in ## Tier 4 — AI-assisted admin endpoints (all behind `RequireAdmin`) -Every endpoint in this tier stays read-only/surface-only, no -exceptions — see `CODEX.md`'s hard rule at the top. +> **Status: Stage 1 and Stage 2 are both built on +> `feat/tier4-ai-admin-endpoints`.** The weekly digest and its schedule +> and history, the support-ticket assistant, the config tuning advisor, +> the LLM provider config, the read-only database provider config and the +> ask-ai widget config all exist, are wired in `main.go`, and are tested +> end to end on the in-memory stores — `go build`/`go vet`/`go test ./...` +> clean, `httpapi` also green under `-race`. +> +> What is still owed, said plainly, because none of it is a small +> caveat: +> +> - **No migration in this tier has ever been applied to a database.** +> There is still no Postgres in this sandbox, so `012_digest_runs` and +> `013_settings` have only been reasoned about, not run — the same is +> true of `009`–`011`. Every `PostgresStore` added here is unexercised. +> - **`aiprovider.CheckReadOnly` has never run against a real Postgres.** +> The probe is a `CREATE TEMP TABLE` plus an `INSERT`, and the branch +> that matters — SQLSTATE 42501 arriving as a `*pq.Error` — has only +> been tested against a closed port, which is the *unverifiable* +> outcome rather than the pass. The accepting path is the one no test +> here covers. +> - **The Anthropic provider has never called Anthropic.** It is tested +> against a local `httptest` server in the Messages API's wire shape, +> which pins the request this repo builds and the response it parses, +> but it is not evidence that the live service agrees. +> - **The ask-ai widget has no serving endpoint.** Stage 2 stores its +> embed and scope configuration and enforces the scope in +> `aiprovider.ScopedProvider`; nothing yet calls `widget.Ask`. So +> `allowed_origins` is recorded and validated but nothing consults it +> at request time, and the GET response deliberately carries no embed +> snippet, because the URL in one would name a route this repo does +> not serve. +> +> What is still owed from Stage 1: +> +> - the digest schedule is a goroutine on `context.Background()`, because +> this repo still has no graceful shutdown. +> +> Three things this tier changed that were not in the spec below, all +> recorded because they are behaviour rather than plumbing: +> +> - **`LOCKOUT_THRESHOLD`/`LOCKOUT_DURATION_MINUTES` are now passed to +> the engine.** cryden does not default these — it reads whatever it +> is handed, and `0`/`0` means an account is locked on its first +> failed password until an instant already past, which is to say +> never. Until this tier the engine ran with both at zero. So this is +> a real behaviour change, not a tidy-up, and it is why the tuning +> report can quote the lockout settings in force rather than cryden's +> documented defaults. +> - **`GET /v1/admin/digest` records nothing.** The spec puts scheduling +> and history in this repo, and that is still exactly where the +> writing happens — but the on-demand endpoint deliberately does not +> write a row, so an operator hitting it twenty times does not fill +> the history with twenty near-identical reports. Only the scheduled +> job writes. +> - **`/v1/admin/settings/*` is the admin surface's first write**, and +> the read-only rule below has been read as covering the AI *tools* +> rather than every route under `/v1/admin`. The reasoning is in +> `SettingsHandlers`' doc comment and in `CLAUDE.md`'s own wording: a +> settings save is what "a human still has to explicitly save that +> change through the normal config UI" names, and no AI-assisted +> handler holds a reference to it. The alternative reading — store the +> LLM key in cryden, or in the environment only — is worse: the spec +> below explicitly says this repo owns that config storage. +> +> The two decisions this tier had recorded as open were resolved by +> following this file's own instruction to make the reasonable call and +> note it: the live provider is built on the **official Anthropic Go +> SDK** rather than hand-rolled HTTP, and the settings credentials use a +> **dedicated `SETTINGS_ENCRYPTION_KEY`** rather than reusing cryden's +> `ENCRYPTION_KEY`, matching this repo's existing convention of +> purpose-specific keys (`CLOUD_LOG_HASH_KEY`). + +Every AI-assisted endpoint in this tier is read-only by construction — +see `CLAUDE.md`'s hard rule at the top. The settings routes at the end of +this list are not AI-assisted endpoints: they are the settings save those +tools' suggestions pre-fill. - **Weekly digest**: `GET /v1/admin/digest` → `cryden.WeeklyDigest`/ `DigestSince`. Plus **scheduling and history** (new, this repo's own @@ -318,6 +397,15 @@ exceptions — see `CODEX.md`'s hard rule at the top. at-rest encryption — treat this credential with the same care as `JWT_SECRET`). This repo then constructs the real `ai.LLMProvider` implementation from that stored config at startup or on change. + **Built, with one piece of this bullet not done.** The endpoints + exist, `DELETE` was added alongside `GET`/`PUT` (a settings screen + with no way to clear a credential is a screen an operator cannot + leave), and `aiprovider.NewAnthropic` is the real implementation, + built on the official Anthropic Go SDK. What is **not** built is the + last sentence: nothing reads the stored config and constructs a + provider from it, because nothing consumes one yet — the widget's + serving endpoint does not exist. The glue lands with its first + caller rather than before it, so it is not written blind. - **Database Provider config** (new): same shape, for pointing `ai.QueryableStore` at a read-only database role/connection string. **The read-only-role requirement is not optional** — cryden's own @@ -327,10 +415,30 @@ exceptions — see `CODEX.md`'s hard rule at the top. role is actually read-only before accepting it if there's any feasible way to check (e.g. attempt a write and confirm it's rejected), don't just trust a checkbox in the UI. + **Built.** `PUT` connects with the supplied credentials and refuses + to store anything until the server has rejected a write on that + connection — see `aiprovider.CheckReadOnly`. Two outcomes are + distinguished that the bullet does not mention, because they call + for different words: a role that *can* write, and a check that could + not reach a conclusion. The second is refused too, since treating it + as a pass would make the check succeed exactly when it is least able + to tell. `aiprovider.NewPostgresSnapshot` is the matching + `ai.QueryableStore`; like the provider above, nothing constructs it + from the stored config yet, for the same reason. - **Ask-AI widget embed/scope config** (new): once the two providers above exist, `widget.Ask` itself needs no new engine work — expose whatever embed snippet / scope configuration the csax+ console needs as its own settings endpoint. + **Built, with two deliberate departures.** The endpoint stores the + widget's enabled flag, origins, entity scope and copy. + `aiprovider.ScopedProvider` then *enforces* the entity scope — + cryden's `widget.Ask` scopes every intent to the calling end user but + does so over all of `ai.AllowedEntities`, so narrowing that is a host + decision and a scope setting nothing consulted would be worse than no + setting. And the response carries **no embed snippet**: the snippet is + markup the console renders into its own pages, and the URL in one + would name a route this repo does not serve. The console gets the + configuration a snippet is built from instead. --- diff --git a/docs/development/PROGRESS.md b/docs/development/PROGRESS.md index 90a545d..322ff4e 100644 --- a/docs/development/PROGRESS.md +++ b/docs/development/PROGRESS.md @@ -35,14 +35,14 @@ Assumptions made, none blocking: operator all get the identical `403 not_operator` — that distinction is not something to expose to the caller. -Next: Tier 1 (auth methods), each on its own branch per `CODEX.md`. +Next: Tier 1 (auth methods), each on its own branch per `CLAUDE.md`. Copying cryden's migrations `0003`-`0007` into this repo (renumbered continuing from `003_operators`) is the first sub-step, before any TOTP/WebAuthn/magic-link/recovery-code endpoint work starts. ## 2026-09-14 — Tier 1 (auth methods) except Apple -Branch `feat/tier1-auth-methods`, per `CODEX.md`'s one-branch-per-tier +Branch `feat/tier1-auth-methods`, per `CLAUDE.md`'s one-branch-per-tier rule. First session in this repo with a working Go toolchain: Go 1.25.0 plus cryden v2.5.0 and every dependency already in the module cache, so the caveat the Tier 0 entry left open is closed — `go mod tidy` @@ -52,7 +52,7 @@ was verified: **the DB-backed smoke test was not run** (no Postgres and no network in this sandbox) and neither were the WebAuthn ceremonies, which need a real browser authenticator. Those still owe a first run against a real database. Saying that plainly here rather than counting -green builds as "verified end to end", per `CODEX.md`. +green builds as "verified end to end", per `CLAUDE.md`. Built, in commit order: @@ -195,12 +195,12 @@ network), the DB-backed smoke test (no Postgres), and the WebAuthn ceremonies (no browser authenticator). Those remain the first things to run on a real deployment. -Next: Tier 2, on its own branch per `CODEX.md` — and before or alongside +Next: Tier 2, on its own branch per `CLAUDE.md` — and before or alongside it, the first DB-backed smoke-test run of everything in Tier 1. ## 2026-09-15 — Tier 2 (config, named sessions, OAuth health) -Branch `feat/tier2-config-and-oauth-health`, per `CODEX.md`'s +Branch `feat/tier2-config-and-oauth-health`, per `CLAUDE.md`'s one-branch-per-tier rule. Three commits, in order: - `feat: wire anomaly detection and the Redis rate limiter from env` — @@ -319,7 +319,7 @@ rather than silently patched): smoketest does not have). An optional operator token/email flag would fix it if that coverage is wanted later. -Next: Tier 3, on its own branch per `CODEX.md`. Still owed from before +Next: Tier 3, on its own branch per `CLAUDE.md`. Still owed from before it: the first DB-backed smoke-test run, now worth doing against a `REDIS_URL`-less and a `REDIS_URL`-set instance so the shared limiter gets its first real exercise. @@ -382,7 +382,7 @@ trailing `// dev stand-in` comments, which align against the longest line in their group. Both fixed with `gofmt -w`. **What was checked before the toolchain was reachable** — since -`CODEX.md`'s rule is to say what was and was not done rather than to +`CLAUDE.md`'s rule is to say what was and was not done rather than to imply a build — every cryden symbol Stage 1 calls was read directly out of the module cache at `…/cryden/v2@v2.5.0`, first-hand, not recalled. Confirmed: @@ -626,7 +626,7 @@ so the field is `Errors`. ### Verification: what this does NOT cover -Said plainly, per `CODEX.md`, rather than implied by a green suite: +Said plainly, per `CLAUDE.md`, rather than implied by a green suite: - **There is no Postgres and no network in this sandbox.** `migrations/009`, `010` and `011` have **never been applied to a real @@ -678,3 +678,330 @@ smuggled in behind the other. Tier 3 is complete. Next is Tier 4, which stays read-only by construction with the pre-fill-never-auto-apply decision already made. + +## 2026-09-15 — Tier 4, Stage 1 (digest, support diagnosis, config tuning) + +Tier 4 is split for the same reason Tier 3 was: the first half is three +read-only reports with their decisions already made in `NEXT.md`, and +the second half needs two decisions that are not this session's to +make (see "Stage 2" below). Branch `feat/tier4-ai-admin-endpoints`. + +Three commits, one logical step each: the digest and its history +(`21ac94c`), the support-ticket login diagnosis (`705b820`), and the +config tuning advisor (`d74d8a4`). + +What each one is, and the one thing about it worth knowing: + +- **`GET /v1/admin/digest`** and **`GET /v1/admin/digest/history`**. + `digest/` is a new repo-owned package (interface + `PostgresStore` + + in-memory double in one file, the convention every store here + follows) over `migrations/012_digest_runs`. The on-demand endpoint + **records nothing**: an operator hitting it twenty times should not + fill a history with twenty near-identical reports, so only the + scheduled job writes. The schedule is this repo's own — cryden has no + concept of one — and the first run lands one full interval after + startup, not at boot, because a process that restarts more often than + the interval elapses would otherwise write a row per restart. +- **`GET /v1/admin/support/diagnose?email=`** → `cryden.DiagnoseLoginIssue`. + An unknown account is an **answer** (`Found:false`), not a 404 or a + 500: "we have never seen this address" is exactly what a support + ticket needs to be told, and dressing it up as a server error would + hide it. +- **`GET /v1/admin/config-tuning`** → `admin.BuildTuningReport` called + **directly**, not `cryden.ConfigTuningReport`. The structured + `TuningSuggestion{Area, Finding, Suggestion}` list is the point: a + console renders one card per suggestion, and a pre-rendered text blob + cannot be turned back into cards. The counts are returned raw + alongside, so the evidence is visible rather than a sentence asking + to be trusted. + +### The lockout passthrough is a behaviour change, not a tidy-up + +Building the tuning advisor surfaced something: **this repo was never +passing `LockoutThreshold`/`LockoutDuration` to the engine.** `config` +had no such fields, so `main.go` left them at Go's zero values, so the +engine ran with a threshold of 0 and a duration of 0 — and cryden does +no defaulting of either. A zero threshold locks an account on its very +first failed password; a zero duration locks it until an instant +already past, which is to say not at all. Every deployment of this API +so far has been in that second state. + +The report is what made it visible: `BuildTuningReport` is asked to +judge the audit history against "the settings in force", and the +settings in force were not what anyone thought they were. So `config` +gained `LockoutThreshold`/`LockoutDuration` (defaulting to cryden's own +5 and 15 minutes, with the values written down here for the same reason +the rate-limit bounds are), `main.go` passes them, and a threshold +below 1 is a **startup error** rather than being read as "off" — +cryden has no way to switch lockout off, so accepting 0 would be +accepting a setting that means something else. + +This is called out in `README.md`, `.env.example` and the commit +message because it changes what every existing deployment does the next +time it restarts. It is the right direction — an account that can be +guessed at forever was not a design decision anyone made — but it is a +change nobody asked for, and burying it in a commit about a reporting +endpoint would have been the wrong way to ship it. + +### Verification: what this does NOT cover + +- **`migrations/012_digest_runs` has never been applied to a + database**, the same as `009`–`011`. Everything above is tested + through the in-memory doubles. +- **`digest.PostgresStore`'s `List` has not been run.** Its limit + clamp is asserted through the in-memory double and through + `ClampLimit` directly, which is the shared rule — but the SQL that + applies it is a copy of a design, not a verified query. The + TIMESTAMPTZ round trip in particular cannot be reproduced by a double + that stores `time.Time` as `time.Time`. +- **`memory.AuditStore` stamps `time.Now()` with no injectable clock**, + so window-*boundary* exclusion cannot be driven through the endpoint. + The digest and tuning tests assert the positive direction (events + recorded moments ago do appear inside a one-day window) and the text + the engine actually renders, rather than backdating an event. +- **The digest schedule is a goroutine on `context.Background()`.** The + scheduler takes a `context.Context` and is tested with a real + cancellable one, but `main.go` has nothing to cancel it with, because + this repo still has no graceful shutdown — the debt Stage 1 of Tier 3 + flagged, now with one more holder. +- **No live LLM call and no live database provider exist to test**, + because Stage 2 is not built. Nothing in Stage 1 touches + `ai.LLMProvider` or `ai.QueryableStore`. +- **`internal/smoketest` still has never been run** against a database, + unchanged from every previous tier's note. + +### Stage 2, and the two decisions it needs + +Stage 2 is the LLM provider config, the database provider config and +the ask-AI widget config. `NEXT.md` settles the shape of all three +(settings endpoints, this repo's own config table, pre-fill never +auto-apply, validate the read-only role by attempting a write). Two +things it does not settle, both of which change what gets built: + +1. **Whether this repo ships a live LLM client at all.** `ai.LLMProvider` + is an interface; implementing it against a real vendor means an + outbound HTTP client, a vendor choice, and a credential that leaves + the building. A console that configures a provider it cannot call is + not useful, so this is likely yes — but it is an integration + decision, not a wrapper decision, and this repo has so far shipped + no outbound integration of its own (the webhooks are cryden calling + a URL this repo hands it). +2. **Where the at-rest encryption key comes from.** `NEXT.md` requires + the stored provider credential be encrypted at rest and treated with + the same care as `JWT_SECRET`. `ENCRYPTION_KEY` already exists and + already encrypts TOTP secrets, so reusing it is the obvious + candidate — but reusing one key across two purposes is a decision + with a blast radius, and the alternative (a second key, or a KMS) + is a deployment change. + +Both were left for the user rather than guessed at. + +### Noticed while working, not fixed + +- **`openapi/spec.yaml` is now at 1.4 and covers Tiers 1–4 Stage 1**, + which closes the gap every previous entry flagged — `NEXT.md`'s Tier 1 + note that the spec "still predates Tier 1" is no longer true. The + document is large and hand-maintained, so it can drift again. +- **The unconfigured-store answer is still `404 not_configured`**, now + used by the digest history and the tuning endpoint too. Consistent + with every other unconfigured feature here, and still + indistinguishable from "this resource genuinely does not exist". +- **`config.Load` now refuses three knobs at startup** that it used to + accept silently (`LOG_LEVEL`, `LOCKOUT_THRESHOLD`, `DIGEST_INTERVAL_HOURS`). + That is the intended direction — a setting that silently does nothing + is worse than one that refuses to start — but it means an existing + deployment with a typo in one of them will fail to boot rather than + run with a default. + +## 2026-09-16 — Tier 4, Stage 2 (LLM provider, read-only DB, ask-ai widget) + +Stage 2 of Tier 4, on the same branch. Three settings endpoints, two new +packages of this repo's own, and the point where this repo stopped being +purely an HTTP wrapper. + +Commits, in order: + +- `bf1abaa` — the settings store and its at-rest encryption + (`settings/`, `migrations/013_settings`, `SETTINGS_ENCRYPTION_KEY`). +- `bc7de0e` — `aiprovider.NewAnthropic`, a live `ai.LLMProvider` over + the official Anthropic Go SDK. +- `b31ef89` — `aiprovider.PostgresSnapshot` plus `CheckReadOnly`. +- `2815e90` — the LLM and database provider endpoints. +- `10cea8d` — the ask-ai widget config and `aiprovider.ScopedProvider`. +- plus an `openapi` bump to 1.5 and a README section. + +### The two decisions the Stage 1 entry left open were taken + +Both by following `NEXT.md`'s own instruction — "where something is +genuinely unspecified, make the most reasonable call consistent with +`CLAUDE.md`'s ownership rules and note the assumption in `PROGRESS.md`" +— rather than by asking, since the instruction to ask was absent and the +spec was explicit that it should not be needed. + +1. **A live LLM client, yes, and on the official SDK.** Implementing + `ai.LLMProvider` was unavoidable: the spec says the console + configures a provider, and a console configuring a provider nothing + can call is not useful. The SDK over hand-rolled HTTP because a + hand-rolled client would be a second thing to keep correct against a + moving API, and because it is the one part of this repo whose + correctness cannot be checked by reading it. +2. **A dedicated `SETTINGS_ENCRYPTION_KEY`, not a reuse of + `ENCRYPTION_KEY`.** The precedent is already in this repo: + `CLOUD_LOG_HASH_KEY` exists rather than reusing `JWT_SECRET`. The two + seal different things with different lifetimes and blast radii — + cryden's key covers what the engine stores, this one what the API + stores — so one rotating should not force the other. Reusing it would + also mean a TOTP-secret rotation and an API-key rotation cannot be + scheduled apart. + +The encryption itself is cryden's `security.NewAESGCMEncryptor`, not a +second AES-GCM implementation. That is the "if cryden already answers +the question, call it" rule applied to a primitive: there is nothing +about a provider API key that needs different treatment from a TOTP +secret, and two implementations of the same cipher is one more place for +a nonce to be reused. + +### The read-only check is the one piece of this tier worth reading twice + +`PUT /v1/admin/settings/database-provider` validates the DSN's shape, +then **connects with the supplied credentials and attempts a write**, +and stores nothing unless the server refuses. cryden's own interface +comment is the requirement ("a real credential-level guarantee, not just +a promise made in code, so a bug in validation still can't cause a +write"), and `NEXT.md` says not to trust a checkbox. Three details are +load-bearing: + +- The probe writes to `pg_temp`, the session's own temporary schema, so + a probe that fails leaves nothing for an operator to clean up. The + pool is pinned to one connection so the `CREATE` and the `INSERT` + share the session that owns the temp table — a pool that split them + would have the `INSERT` fail on a missing table, which looks exactly + like the refusal being tested for and is not one. +- Only SQLSTATE `42501` counts as a refusal, matched by code rather than + by message, because the message is localized and reworded between + major versions and this is the branch that decides acceptance. +- A third outcome is distinguished from both: a connection that never + opened, a timeout, or a `CREATE` that failed for a non-privilege + reason is `database_role_unverified` and **is refused**. Treating + "could not find out" as a pass would make the check succeed precisely + when it is least able to tell. + +The cost is that this endpoint is slow relative to its neighbours — +a connection and two statements — bounded by a ten-second timeout. That +is once per save, not once per query. + +### The widget's entity scope has teeth, on purpose + +`widget.Ask` force-scopes every parsed intent to the calling end user's +own rows, overwriting rather than validating the identity filter the +model produced (no oracle: every phrasing executes the same query). But +it scopes over all of `ai.AllowedEntities`. Narrowing that is a host +decision — cryden's allowlist is "what can be scoped safely", the +operator's is "what this deployment offers" — so this repo enforces the +configured subset in `aiprovider.ScopedProvider`, in front of the +provider, which is the only place the entity is still visible before +`Ask` parses, scopes and executes in one call. The alternative was +storing a scope setting nothing consulted, which is worse than not +having the setting. + +The list is checked against cryden's own `AllowedEntities` map rather +than a copy, so this repo cannot refuse an entity the engine permits. +One asymmetry is accepted knowingly and commented: `scopeToOwner` is a +private switch over today's three entities, so an entity added to +cryden's allowlist without a matching case there would pass validation +and then fail at `Ask` time with `ErrEntityNotAvailable`. That is the +safe direction — the widget refuses the question rather than answering +it unscoped. + +### Verification: what this does NOT cover + +The suite is green — `gofmt -l` clean, `go build ./...`, `go vet ./...`, +`go test -count=1 ./...` all pass, and `httpapi`, `settings` and +`aiprovider` also pass under `-race`. That is not the same as this +working, and three specific things are unproven: + +- **`aiprovider.CheckReadOnly` has never run against a real Postgres.** + There is no Postgres in this sandbox. `query_test.go` covers the + statement builder, the filter allowlist, the LIKE escaping and the + probe statements' shape, and `TestPutDatabaseProviderRefusesAnUnverifiableConnection` + drives a real connection attempt — but against a *closed port*, which + exercises the unverifiable branch. **The accepting path — 42501 + arriving as a `*pq.Error` and being read as a pass — is the branch no + test here covers**, and it is the branch the feature depends on. + Likewise nothing has confirmed that a `CREATE TEMP TABLE` is actually + refused by a `GRANT SELECT`-only role as opposed to failing some other + way, which is the assumption the probe is built on. +- **The Anthropic provider has never called Anthropic.** It is tested + against a local `httptest` server in the Messages API's wire shape, + which pins the request this repo builds and the response it parses. + That is a real test of this repo's half and no evidence at all about + the live service's half: a model id, a schema field name or a refusal + shape that differs in production would not be caught. +- **`012_digest_runs` and `013_settings` have never been applied to a + database**, the same as `009`–`011`. Every `PostgresStore` in + `settings/` and `digest/` is reasoned-about rather than run, so a + column type or a constraint error would surface at first deploy. + +Also unchanged from every previous tier: `internal/smoketest` has still +never been run. + +### Two things deliberately not built, rather than half-built + +- **Nothing constructs `ai.LLMProvider` or `ai.QueryableStore` from the + stored config.** `NEXT.md` asks for it ("this repo then constructs the + real implementation from that stored config at startup or on change"). + The glue's only possible consumer today is a widget serving endpoint, + which does not exist, so writing it now would mean writing the + consumer's half blind and then rewriting it. It lands with its first + caller. +- **The widget GET returns no embed snippet.** The snippet is markup the + console renders into its own pages, and its `