Keep a newly seeded model key ahead of a stale secret
ci / vet-and-test (pull_request) Successful in 1m1s
ci / vet-and-test (pull_request) Successful in 1m1s
Why: The active secret_keys row overrides settings plaintext, so a replaced chat key kept returning 401. Migration also mutated the settings map in place, so clearing a matching plaintext never reached the database. Evidence: go test ./internal/settings/ ./internal/runtime/ -count=1 (SELF, worktree). Impact: System settings writes adopt a changed secret; conflicting plaintext is kept and logged; 401 errors name the key layer and an 8-hex fingerprint. Refs: fix/model-key-secret-overlay Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
+6
-3
@@ -292,16 +292,19 @@ func Run(opt Options) error {
|
||||
if err := setSvc.SnapshotAndMigrateSecrets(); err != nil {
|
||||
return fmt.Errorf("migrate secrets: %w", err)
|
||||
}
|
||||
runtime.SecretLookup = func(owner, service string) (string, bool) {
|
||||
runtime.SecretLookup = func(owner, service string) (string, string, bool) {
|
||||
if owner == "" {
|
||||
owner = store.OwnerDefault
|
||||
}
|
||||
if v, ok := st.ActiveSecret(owner, service); ok {
|
||||
return v, true
|
||||
return v, "owner", true
|
||||
}
|
||||
// P2 受管:密钥统一迁入 secrets 表后,系统层受管密钥挂在系统行
|
||||
//(owner=settings.OwnerSystemRow);租户未单独配置时回退系统密钥。
|
||||
return st.ActiveSecret(settings.OwnerSystemRow, service)
|
||||
if v, ok := st.ActiveSecret(settings.OwnerSystemRow, service); ok {
|
||||
return v, "system", true
|
||||
}
|
||||
return "", "", false
|
||||
}
|
||||
rag.SecretLookup = func(service, fallback string) string {
|
||||
return runtime.LookupSecret(store.OwnerDefault, service, fallback)
|
||||
|
||||
@@ -0,0 +1,48 @@
|
||||
package runtime
|
||||
|
||||
import (
|
||||
"context"
|
||||
"crypto/sha256"
|
||||
"encoding/hex"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
agentllm "agent-native/runtime/llm"
|
||||
core "agent-system-core"
|
||||
)
|
||||
|
||||
func TestUnauthorizedNamesKeySourceWithoutRevealingIt(t *testing.T) {
|
||||
const secret = "sk-owner-secret"
|
||||
SecretLookup = func(owner, service string) (string, string, bool) {
|
||||
if owner == "weichen" && service == "models.chat" {
|
||||
return secret, "owner", true
|
||||
}
|
||||
return "", "", false
|
||||
}
|
||||
t.Cleanup(func() { SecretLookup = nil })
|
||||
|
||||
route := ResolveChatModel(core.AgentProfile{Owner: "weichen"}, effOf(map[string]any{
|
||||
"models": map[string]any{"chat": map[string]any{"model": "m", "baseUrl": "https://gw", "apiKey": "sk-settings"}},
|
||||
}))
|
||||
if route.Key != secret || route.KeySource != "owner" {
|
||||
t.Fatalf("route = %+v", route)
|
||||
}
|
||||
|
||||
msg := "provider status 401: invalid api key"
|
||||
stream := annotateKeyFailure(func(context.Context, string, []agentllm.Message, []agentllm.ToolDecl, agentllm.StreamOptions) <-chan agentllm.Event {
|
||||
ch := make(chan agentllm.Event, 1)
|
||||
ch <- agentllm.Event{
|
||||
Kind: agentllm.KindError, HTTPStatus: 401, ErrorMessage: msg,
|
||||
Err: &agentllm.StreamError{Message: msg},
|
||||
}
|
||||
close(ch)
|
||||
return ch
|
||||
}, route)
|
||||
ev := <-stream(context.Background(), "m", nil, nil, agentllm.StreamOptions{})
|
||||
got := ev.Err.Error()
|
||||
sum := sha256.Sum256([]byte(secret))
|
||||
fp := hex.EncodeToString(sum[:4])
|
||||
if strings.Contains(got, secret) || !strings.Contains(got, "source=owner fp="+fp) || !strings.Contains(ev.ErrorMessage, "source=owner fp="+fp) {
|
||||
t.Fatalf("error = %s", got)
|
||||
}
|
||||
}
|
||||
@@ -1,6 +1,8 @@
|
||||
package runtime
|
||||
|
||||
import (
|
||||
"context"
|
||||
"crypto/sha256"
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"math"
|
||||
@@ -249,6 +251,7 @@ func (l *LLMRunner) Execute(rc RunCtx) error {
|
||||
client := openLLMClient(route.Base, route.Key, route.Model)
|
||||
stream = client.Stream
|
||||
}
|
||||
stream = annotateKeyFailure(stream, route)
|
||||
|
||||
// 知识注入:当前消息包含 @knowledge 引用时检索注入(最小实现:显式引用)。
|
||||
var knowledge []core.RetrievalHit
|
||||
@@ -494,6 +497,53 @@ func agentNameOf(prof core.AgentProfile) string {
|
||||
return "agent-system"
|
||||
}
|
||||
|
||||
// annotateKeyFailure 在 401 的错误文本后附上密钥来源和脱敏指纹,不附明文。
|
||||
func annotateKeyFailure(stream agentllm.StreamFunc, route ResolvedModel) agentllm.StreamFunc {
|
||||
if stream == nil {
|
||||
return nil
|
||||
}
|
||||
return func(ctx context.Context, model string, messages []agentllm.Message, tools []agentllm.ToolDecl, opts agentllm.StreamOptions) <-chan agentllm.Event {
|
||||
out := make(chan agentllm.Event)
|
||||
go func() {
|
||||
defer close(out)
|
||||
for ev := range stream(ctx, model, messages, tools, opts) {
|
||||
if ev.HTTPStatus == http.StatusUnauthorized {
|
||||
ev = withKeyProvenance(ev, route)
|
||||
}
|
||||
select {
|
||||
case <-ctx.Done():
|
||||
return
|
||||
case out <- ev:
|
||||
}
|
||||
}
|
||||
}()
|
||||
return out
|
||||
}
|
||||
}
|
||||
|
||||
func withKeyProvenance(ev agentllm.Event, route ResolvedModel) agentllm.Event {
|
||||
note := keyProvenance(route)
|
||||
ev.ErrorMessage += note
|
||||
switch err := ev.Err.(type) {
|
||||
case *agentllm.StreamError:
|
||||
err.Message += note
|
||||
case nil:
|
||||
ev.Err = fmt.Errorf("%s", strings.TrimSpace(note))
|
||||
default:
|
||||
ev.Err = fmt.Errorf("%s%s", err.Error(), note)
|
||||
}
|
||||
return ev
|
||||
}
|
||||
|
||||
func keyProvenance(route ResolvedModel) string {
|
||||
sum := sha256.Sum256([]byte(route.Key))
|
||||
source := route.KeySource
|
||||
if source == "" {
|
||||
source = "settings"
|
||||
}
|
||||
return fmt.Sprintf(" (key source=%s fp=%x)", source, sum[:4])
|
||||
}
|
||||
|
||||
// openLLMClient 不设 HTTP 客户端超时。单次生成可能很长,停止按钮通过 run 的 context 取消。
|
||||
func openLLMClient(base, key, model string) *agentllm.Client {
|
||||
c := &agentllm.Client{BaseURL: base, APIKey: key, HTTPClient: &http.Client{}}
|
||||
|
||||
+22
-17
@@ -7,25 +7,29 @@ import (
|
||||
core "agent-system-core"
|
||||
)
|
||||
|
||||
// SecretLookup 由组合根注入。返回该服务当前生效的明文;没有独立密钥时 ok 为 false,调用方用设置里的旧字段。
|
||||
var SecretLookup func(owner, service string) (string, bool)
|
||||
// SecretLookup 由组合根注入。layer 是 owner 或 system。没有独立密钥时 ok 为 false,调用方用设置里的旧字段。
|
||||
var SecretLookup func(owner, service string) (value, layer string, ok bool)
|
||||
|
||||
// LookupSecret 给组合根以外的调用方使用。owner 为空时用 local。
|
||||
func LookupSecret(owner, service, fallback string) string {
|
||||
return overlayKey(owner, service, fallback)
|
||||
value, _ := overlayKey(owner, service, fallback)
|
||||
return value
|
||||
}
|
||||
|
||||
func overlayKey(owner, service, fallback string) string {
|
||||
func overlayKey(owner, service, fallback string) (string, string) {
|
||||
if SecretLookup == nil {
|
||||
return fallback
|
||||
return fallback, "settings"
|
||||
}
|
||||
if owner == "" {
|
||||
owner = "local"
|
||||
}
|
||||
if v, ok := SecretLookup(owner, service); ok && strings.TrimSpace(v) != "" {
|
||||
return v
|
||||
if v, layer, ok := SecretLookup(owner, service); ok && strings.TrimSpace(v) != "" {
|
||||
if layer == "" {
|
||||
layer = "owner"
|
||||
}
|
||||
return v, layer
|
||||
}
|
||||
return fallback
|
||||
return fallback, "settings"
|
||||
}
|
||||
|
||||
// 模型路由解析(4.7 / 4.9 覆盖链):设置层 defaults → system → user → project,
|
||||
@@ -45,10 +49,11 @@ type ContextInjection struct {
|
||||
|
||||
// ResolvedModel 是一次路由解析的结果。
|
||||
type ResolvedModel struct {
|
||||
Kind string // chat | lightweight
|
||||
Model string
|
||||
Base string
|
||||
Key string
|
||||
Kind string // chat | lightweight
|
||||
Model string
|
||||
Base string
|
||||
Key string
|
||||
KeySource string // owner | system | settings
|
||||
}
|
||||
|
||||
// ResolveChatModel 解析主对话模型(画像 route > 设置层)。
|
||||
@@ -57,9 +62,9 @@ func ResolveChatModel(prof core.AgentProfile, eff core.EffectiveSettings) Resolv
|
||||
if prof.Model.Chat != "" {
|
||||
m = prof.Model.Chat
|
||||
}
|
||||
key, source := overlayKey(prof.Owner, "models.chat", eff.GetString("models.chat.apiKey", ""))
|
||||
return ResolvedModel{Kind: "chat", Model: m,
|
||||
Base: eff.GetString("models.chat.baseUrl", ""),
|
||||
Key: overlayKey(prof.Owner, "models.chat", eff.GetString("models.chat.apiKey", ""))}
|
||||
Base: eff.GetString("models.chat.baseUrl", ""), Key: key, KeySource: source}
|
||||
}
|
||||
|
||||
// ResolveVisionModel 在当前轮含图片时核对视觉清单。清单内的模型保持原选择;
|
||||
@@ -142,7 +147,7 @@ func ChatFallbacksFor(eff core.EffectiveSettings, hasImages bool) []string {
|
||||
func ResolveLightweightModel(prof core.AgentProfile, eff core.EffectiveSettings) ResolvedModel {
|
||||
m := eff.GetString("models.lightweight.model", "")
|
||||
base := eff.GetString("models.lightweight.baseUrl", "")
|
||||
key := overlayKey(prof.Owner, "models.lightweight", eff.GetString("models.lightweight.apiKey", ""))
|
||||
key, source := overlayKey(prof.Owner, "models.lightweight", eff.GetString("models.lightweight.apiKey", ""))
|
||||
if prof.Model.Lightweight != "" {
|
||||
m = prof.Model.Lightweight
|
||||
}
|
||||
@@ -153,8 +158,8 @@ func ResolveLightweightModel(prof core.AgentProfile, eff core.EffectiveSettings)
|
||||
m = chat.Model
|
||||
}
|
||||
if base == "" {
|
||||
base, key = chat.Base, chat.Key
|
||||
base, key, source = chat.Base, chat.Key, chat.KeySource
|
||||
}
|
||||
}
|
||||
return ResolvedModel{Kind: "lightweight", Model: m, Base: base, Key: key}
|
||||
return ResolvedModel{Kind: "lightweight", Model: m, Base: base, Key: key, KeySource: source}
|
||||
}
|
||||
|
||||
@@ -2,6 +2,7 @@ package settings
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"log"
|
||||
"strings"
|
||||
"time"
|
||||
)
|
||||
@@ -20,7 +21,8 @@ var secretSpecs = []secretSpec{
|
||||
}
|
||||
|
||||
// SnapshotAndMigrateSecrets 先为每个已存 scope 留快照,再把明文密钥迁入独立表并从表单设置里清掉。
|
||||
// 该服务已经有密钥行时不再迁入,避免重复。掩码占位符不迁。
|
||||
// 没有生效密钥时迁入。生效密钥与明文相同则只清明文。两边不同则留下明文并告警,不覆盖生效密钥。
|
||||
// 掩码占位符不迁。
|
||||
func (s *Service) SnapshotAndMigrateSecrets() error {
|
||||
refs, err := s.st.ListSettingsScopes()
|
||||
if err != nil {
|
||||
@@ -43,19 +45,22 @@ func (s *Service) SnapshotAndMigrateSecrets() error {
|
||||
if !ok || val == MaskPlaceholder {
|
||||
continue
|
||||
}
|
||||
has, err := s.st.HasSecret(ref.Owner, spec.service)
|
||||
if err != nil {
|
||||
return err
|
||||
active, hasActive := s.st.ActiveSecret(ref.Owner, spec.service)
|
||||
if hasActive && active != val {
|
||||
// 生效密钥优先于明文。两边不同就留下明文并告警,避免静默丢掉新 key。
|
||||
log.Printf("settings: active secret %s for owner %q differs from stored plaintext; keeping plaintext", spec.service, ref.Owner)
|
||||
continue
|
||||
}
|
||||
if !has {
|
||||
if !hasActive {
|
||||
if _, err := s.st.AddSecret(ref.Owner, spec.service, "迁入", val); err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
path := spec.path
|
||||
if err := s.st.UpdateSettingsScope(ref.Owner, ref.Scope, func(cur map[string]any) (map[string]any, error) {
|
||||
deletePath(cur, path)
|
||||
return cur, nil
|
||||
next := cloneMap(cur)
|
||||
deletePath(next, path)
|
||||
return next, nil
|
||||
}); err != nil {
|
||||
return err
|
||||
}
|
||||
@@ -97,6 +102,40 @@ func (s *Service) snapshotThrottledAccountChecked(actor, owner, scope string, al
|
||||
_, _ = s.st.SaveSnapshotIfChangedAccountChecked(actor, owner, scope, string(raw), "save", allowImplicitDefault)
|
||||
}
|
||||
|
||||
// adoptSystemSecrets 把系统层写入里的明文密钥同步成该服务的生效密钥。
|
||||
// 与当前生效值相同则不追加行。掩码占位符在 prepareWrite 时已经丢掉。
|
||||
func (s *Service) adoptSystemSecrets(owner, scope string, incoming map[string]any) error {
|
||||
if scope != ScopeSystem || owner != OwnerSystemRow {
|
||||
return nil
|
||||
}
|
||||
for _, spec := range secretSpecs {
|
||||
val, ok := lookupString(incoming, spec.path)
|
||||
if !ok || val == MaskPlaceholder {
|
||||
continue
|
||||
}
|
||||
if cur, ok := s.st.ActiveSecret(owner, spec.service); ok && cur == val {
|
||||
continue
|
||||
}
|
||||
if _, err := s.st.AddSecret(owner, spec.service, "播种", val); err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// cloneMap 复制一份设置 map。就地删除后再和原 map 比较会被判定为没变化,写不进库。
|
||||
func cloneMap(m map[string]any) map[string]any {
|
||||
raw, err := json.Marshal(m)
|
||||
if err != nil {
|
||||
return map[string]any{}
|
||||
}
|
||||
var out map[string]any
|
||||
if err := json.Unmarshal(raw, &out); err != nil || out == nil {
|
||||
return map[string]any{}
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
func lookupString(m map[string]any, path string) (string, bool) {
|
||||
var cur any = m
|
||||
for _, p := range strings.Split(path, ".") {
|
||||
|
||||
@@ -0,0 +1,129 @@
|
||||
package settings
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"log"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"agent-system-backend/internal/store"
|
||||
)
|
||||
|
||||
func TestMigrateKeepsPlaintextWhenActiveSecretDiffers(t *testing.T) {
|
||||
st, err := store.OpenSQLite(filepath.Join(t.TempDir(), "secrets.db"))
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
t.Cleanup(func() { st.Close() })
|
||||
svc := NewService(st)
|
||||
if _, err := st.AddSecret(OwnerSystemRow, "models.chat", "迁入", "sk-old"); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := st.MergeSettingsScope(OwnerSystemRow, ScopeSystem, map[string]any{
|
||||
"models": map[string]any{"chat": map[string]any{"apiKey": "sk-new"}},
|
||||
}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
var buf bytes.Buffer
|
||||
prev := log.Default().Writer()
|
||||
log.SetOutput(&buf)
|
||||
t.Cleanup(func() { log.SetOutput(prev) })
|
||||
if err := svc.SnapshotAndMigrateSecrets(); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
cur, err := st.GetSettingsScope(OwnerSystemRow, ScopeSystem)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if got, ok := lookupString(cur, "models.chat.apiKey"); !ok || got != "sk-new" {
|
||||
t.Fatalf("conflicting plaintext = %q ok=%v", got, ok)
|
||||
}
|
||||
active, ok := st.ActiveSecret(OwnerSystemRow, "models.chat")
|
||||
if !ok || active != "sk-old" {
|
||||
t.Fatalf("active secret = %q ok=%v", active, ok)
|
||||
}
|
||||
if !strings.Contains(buf.String(), "models.chat") {
|
||||
t.Fatalf("missing conflict warning: %s", buf.String())
|
||||
}
|
||||
var snaps int
|
||||
if err := st.DB().QueryRow(`SELECT COUNT(*) FROM settings_snapshots WHERE owner=? AND scope=?`, OwnerSystemRow, ScopeSystem).Scan(&snaps); err != nil || snaps == 0 {
|
||||
t.Fatalf("startup snapshot count=%d err=%v", snaps, err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestMigrateClearsPlaintextWhenActiveSecretMatches(t *testing.T) {
|
||||
st, err := store.OpenSQLite(filepath.Join(t.TempDir(), "secrets.db"))
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
t.Cleanup(func() { st.Close() })
|
||||
svc := NewService(st)
|
||||
if _, err := st.AddSecret(OwnerSystemRow, "models.chat", "迁入", "sk-same"); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := st.MergeSettingsScope(OwnerSystemRow, ScopeSystem, map[string]any{
|
||||
"models": map[string]any{"chat": map[string]any{"apiKey": "sk-same", "model": "keep-me"}},
|
||||
}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := svc.SnapshotAndMigrateSecrets(); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
cur, err := st.GetSettingsScope(OwnerSystemRow, ScopeSystem)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if _, ok := lookupString(cur, "models.chat.apiKey"); ok {
|
||||
t.Fatalf("matching plaintext still stored: %#v", cur)
|
||||
}
|
||||
if got, _ := lookupString(cur, "models.chat.model"); got != "keep-me" {
|
||||
t.Fatalf("non-secret field = %q", got)
|
||||
}
|
||||
active, ok := st.ActiveSecret(OwnerSystemRow, "models.chat")
|
||||
if !ok || active != "sk-same" {
|
||||
t.Fatalf("active secret = %q ok=%v", active, ok)
|
||||
}
|
||||
}
|
||||
|
||||
func TestSystemMergeAdoptsChangedSecret(t *testing.T) {
|
||||
st, err := store.OpenSQLite(filepath.Join(t.TempDir(), "secrets.db"))
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
t.Cleanup(func() { st.Close() })
|
||||
svc := NewService(st)
|
||||
if _, err := st.AddSecret(OwnerSystemRow, "models.chat", "迁入", "sk-old"); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := svc.MergeSystem(map[string]any{
|
||||
"models": map[string]any{"chat": map[string]any{"apiKey": "sk-new"}},
|
||||
}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
active, ok := st.ActiveSecret(OwnerSystemRow, "models.chat")
|
||||
if !ok || active != "sk-new" {
|
||||
t.Fatalf("active secret = %q ok=%v", active, ok)
|
||||
}
|
||||
var label string
|
||||
if err := st.DB().QueryRow(`SELECT label FROM secret_keys WHERE owner=? AND service=? AND active=1`, OwnerSystemRow, "models.chat").Scan(&label); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if label != "播种" {
|
||||
t.Fatalf("label = %q", label)
|
||||
}
|
||||
if err := svc.MergeSystem(map[string]any{
|
||||
"models": map[string]any{"chat": map[string]any{"apiKey": "sk-new"}},
|
||||
}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
var n int
|
||||
if err := st.DB().QueryRow(`SELECT COUNT(*) FROM secret_keys WHERE owner=? AND service=?`, OwnerSystemRow, "models.chat").Scan(&n); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if n != 2 {
|
||||
t.Fatalf("secret rows = %d, want the original plus one adoption", n)
|
||||
}
|
||||
}
|
||||
@@ -268,6 +268,9 @@ func (s *Service) mergeScopeAccountChecked(actor, owner, scope string, values ma
|
||||
}); err != nil {
|
||||
return err
|
||||
}
|
||||
if err := s.adoptSystemSecrets(owner, scope, incoming); err != nil {
|
||||
return err
|
||||
}
|
||||
s.snapshotThrottledAccountChecked(actor, owner, scope, allowImplicitDefault)
|
||||
return nil
|
||||
}
|
||||
@@ -286,6 +289,9 @@ func (s *Service) mergeScope(owner, scope string, values map[string]any) error {
|
||||
}); err != nil {
|
||||
return err
|
||||
}
|
||||
if err := s.adoptSystemSecrets(owner, scope, incoming); err != nil {
|
||||
return err
|
||||
}
|
||||
s.snapshotThrottled(owner, scope)
|
||||
return nil
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user