fix(14): WR-04 require the AI key again when the endpoint changes

An admin who cannot read the write-only key could point base_url at
their own host, leave the key redacted and have it sent there on the
next call. An update that keeps the stored key may no longer change the
adapter or base URL; changing either needs the key re-entered.
This commit is contained in:
Jakub Zych
2026-10-04 08:08:07 +02:00
parent 6646d10ff5
commit 93b2b05476
3 changed files with 46 additions and 4 deletions

View File

@@ -23,7 +23,7 @@ The plugin's config keys, lang namespace and plugin ID are a contract shared by
- **OpenAI-compatible adapter.** `providers.OpenAIAdapter` posts to `<base>/chat/completions` with `Authorization: Bearer`, drops `response_format`, maps `max_tokens` to `max_completion_tokens` and adds `reasoning_effort: low` for o-series and gpt-5 models. It also serves compatible providers through their base URL. `providers.ForName` picks the adapter by name.
- **Anthropic adapter.** `providers.AnthropicAdapter` posts to `<base>/messages` with `x-api-key`, `anthropic-version: 2023-06-01` and `anthropic-beta: files-api-2025-04-14`. It sends `max_tokens` (the prompt's `max_tokens`, else `max_completion_tokens`, else 4096), rewrites image blocks to base64 or URL image sources and file blocks to document blocks, and maps Anthropic's usage to `prompt_tokens`, `completion_tokens` and `total_tokens`. `Raw` keeps `stop_reason`.
- **AI service.** `services.AIService.Send` applies the model's system prompt when the prompt has none and posts the prompt to the config's chat endpoint; a nil config uses the default model. It returns the WinterCMS plugin's failure messages: no model configured, no API key, no connection, and an answer that is not JSON. The other calls are `services.AIService.SendStream` (server-sent events, one `{"content":…}` chunk per delta), `services.AIService.SendFile`, `services.AIService.SendFilePath` and `services.AIService.SendFileReader` (multipart upload to the files endpoint of a model with `has_files_endpoint`), `services.AIService.SendWithModel`, `services.AIService.SendToImageModel`, `services.AIService.SendToVisionModel`, `services.AIService.GenerateImage` (the images endpoint; check the returned URL with `security.AssertSafeURL` before fetching it) and `services.AIService.Ask`. `services.ForApp` returns the application's shared service.
- **AI models.** Operators configure models in the admin under Golem › AI Models (controller `golem15.golem.models`, permission `golem15.golem.access_settings`). Each model has a name, an adapter (`openai` or `anthropic`), an API key, a base URL, a model id, a default system prompt, flags (enabled, default, generates images, accepts images, has a files endpoint), a completion token budget and an order. The key is encrypted at rest and write-only: the form shows it redacted, and an update that sends it back empty or redacted keeps the stored key. `services.Models`, `services.DefaultModel`, `services.VisionModel`, `services.ImageModel`, `services.FileModel` and `services.ModelByName` read them like the WinterCMS Settings model. Their configs are `Trusted`. `services.DefaultModel` keeps the WinterCMS quirk: without a model marked default it returns the first model only when that one is enabled.
- **AI models.** Operators configure models in the admin under Golem › AI Models (controller `golem15.golem.models`, permission `golem15.golem.access_settings`). Each model has a name, an adapter (`openai` or `anthropic`), an API key, a base URL, a model id, a default system prompt, flags (enabled, default, generates images, accepts images, has a files endpoint), a completion token budget and an order. The key is encrypted at rest and write-only: the form shows it redacted, and an update that sends it back empty or redacted keeps the stored key, unless the same update changes the adapter or base URL: then the key must be entered again, so a kept key never follows a new endpoint. `services.Models`, `services.DefaultModel`, `services.VisionModel`, `services.ImageModel`, `services.FileModel` and `services.ModelByName` read them like the WinterCMS Settings model. Their configs are `Trusted`. `services.DefaultModel` keeps the WinterCMS quirk: without a model marked default it returns the first model only when that one is enabled.
- **Settings import.** `golem:import-settings` copies the AI models of a WinterCMS installation (the `models` repeater of the `system_settings` row `golem_settings`, or a JSON file) into the table once, encrypting each plaintext key.
- **SSRF guard.** `security.AssertSafeURL` ports the WinterCMS SSRFGuard: https only, a host on the allowlist (`security.AllowedHosts`), and a host that resolves to public addresses only (`fetchguard.IsPrivateAddr`). A failure is a `security.UnsafeURLError` with the WinterCMS message. Apply it to every base URL a user or an organisation supplies and to every URL a provider returns; the guarded client checks the address again when it connects.
- **Prompt factory.** `factories.PromptFactory` builds the WinterCMS plugin's ready-made prompts: a form field suggestion, values for several fields, categories for an item, a chat and a completion.

View File

@@ -3,6 +3,7 @@ package controllers
import (
"context"
"errors"
"strings"
"git.golem15.com/golem15/sm-golem-plugin/models"
"git.golem15.com/golem15/summercms/modules/cabana"
@@ -15,7 +16,7 @@ const PermissionAccessSettings = "golem15.golem.access_settings"
// ModelsAdminController is the AI models list and form
// (golem15.golem.models). The API key is write-only: the form shows it
// redacted, and an update that sends it back empty or redacted keeps the
// stored key.
// stored key, unless the same update changes the adapter or base URL.
type ModelsAdminController struct{}
var (
@@ -64,8 +65,19 @@ func (ModelsAdminController) FormBeforeCreate(_ context.Context, model any) erro
return nil
}
// baseURLOf is a model's base URL with nil and blank alike.
func baseURLOf(m *models.AiModel) string {
if m.BaseURL == nil {
return ""
}
return strings.TrimSpace(*m.BaseURL)
}
// FormBeforeUpdate keeps the stored key when the form sent it back empty
// or redacted.
// or redacted. A kept key never follows a changed endpoint: when the
// adapter or base URL differs from the stored one, the key must be entered
// again, so an operator who cannot read the key cannot redirect it to a
// host they control either.
func (ModelsAdminController) FormBeforeUpdate(ctx context.Context, model any) error {
m, ok := model.(*models.AiModel)
if !ok {
@@ -79,9 +91,12 @@ func (ModelsAdminController) FormBeforeUpdate(ctx context.Context, model any) er
return errors.New("golem: no transaction for the model update")
}
var stored models.AiModel
if err := tx.Select("api_key").Where("id = ?", m.ID).Take(&stored).Error; err != nil {
if err := tx.Select("api_key", "adapter", "base_url").Where("id = ?", m.ID).Take(&stored).Error; err != nil {
return err
}
if stored.Adapter != m.Adapter || baseURLOf(&stored) != baseURLOf(m) {
return &cabana.ValidationError{Details: map[string]any{"api_key": []string{"Re-enter the API key when changing the adapter or base URL."}}}
}
m.APIKey = stored.APIKey
if keyMissing(m) {
return &cabana.ValidationError{Details: map[string]any{"api_key": []string{"The api key field is required."}}}

View File

@@ -4,6 +4,7 @@ import (
"context"
"encoding/json"
"errors"
"maps"
"os"
"path/filepath"
"strings"
@@ -247,6 +248,32 @@ func TestAdminModelsForm(t *testing.T) {
if keyOf() != "sk-ant-admin-0002" {
t.Fatal("a new key was not stored")
}
// A kept key never follows a changed endpoint: changing the base URL
// or the adapter without re-entering the key is refused (WR-04).
for field, value := range map[string]any{"base_url": "https://attacker.example/v1", "adapter": "openai"} {
changed := maps.Clone(body)
changed["api_key"] = "[redacted]"
changed[field] = value
_, err := svc.Update(ctx, cc, id, cabana.RecordInput{Body: changed})
if !errors.As(err, &ve) || ve.Details["api_key"] == nil {
t.Fatalf("a %s change with the redacted key = %v", field, err)
}
if keyOf() != "sk-ant-admin-0002" {
t.Fatalf("the refused %s change touched the key", field)
}
changed["api_key"] = "sk-ant-admin-0003"
if _, err := svc.Update(ctx, cc, id, cabana.RecordInput{Body: changed}); err != nil {
t.Fatalf("a %s change with a new key: %v", field, err)
}
if keyOf() != "sk-ant-admin-0003" {
t.Fatalf("the %s change did not store the new key", field)
}
restore := maps.Clone(body)
restore["base_url"] = ""
if _, err := svc.Update(ctx, cc, id, cabana.RecordInput{Body: restore}); err != nil {
t.Fatalf("restoring the endpoint: %v", err)
}
}
body["adapter"] = "gemini"
if _, err := svc.Update(ctx, cc, id, cabana.RecordInput{Body: body}); !errors.As(err, &ve) {
t.Fatalf("unknown adapter = %v", err)