From dc169b074f017baa2c215a2761a14658379f920d Mon Sep 17 00:00:00 2001 From: BaaRai <130491490+BaaRai@users.noreply.github.com> Date: Fri, 13 Nov 2026 21:33:27 +0100 Subject: [PATCH] secrets: encrypt search keys, proxies and the LLM profile proxy at rest Phase 2 of secrets at rest (#213), same AES-256-GCM scheme and key. - llm_profiles.proxy joins the secret columns (AAD llm_profiles.proxy); scanProfile decrypts it for both column lists, SaveProfile encrypts it, an empty proxy stays NULL - brave_search_api_key, tavily_search_api_key, web_search_proxy and global_proxy are stored encrypted (AAD settings.) through GetSetting, SetSetting and InsertSettingIfAbsent; other settings stay in clear - the startup backfill, the no-key refusal and the wrong-key check cover the new values; legacy plaintext is read as-is until backfill API: the proxies are returned masked as a whole (placeholder plus a *_set flag) by GET /api/settings, GET /api/llm/profiles and GET /api/llm. On write the placeholder keeps the stored value, an explicit value replaces it and "" clears it; the placeholder is never stored. The test and model-list endpoints resolve the placeholder to the stored proxy. --- README.md | 5 +- db/config.go | 15 +++- db/secrets.go | 78 +++++++++++++++---- db/secrets_test.go | 150 ++++++++++++++++++++++++++++++++++--- db/settings.go | 25 ++++++- server/dto.go | 6 +- server/secrets_api_test.go | 115 ++++++++++++++++++++++++++++ server/server.go | 31 +++++++- server/server_mgmt.go | 38 ++++++++++ 9 files changed, 424 insertions(+), 39 deletions(-) diff --git a/README.md b/README.md index ea8ac8e..9069a2c 100644 --- a/README.md +++ b/README.md @@ -181,8 +181,9 @@ gunzip -c backups/artex-db-.sql.gz | docker compose exec -T postgres psql -U "$POSTGRES_USER" -d "$POSTGRES_DB" ``` -Stored secrets (LLM API keys, MCP env, task credentials, notification channel -config) are encrypted in the database with the key in `keys/secret.key` +Stored secrets (LLM API keys and proxies, MCP env, task credentials, +notification channel config, the Brave and Tavily search keys, the web search +and global proxies) are encrypted in the database with the key in `keys/secret.key` (generated at first start, mode 0600, next to `jwt.key`). **A database backup without that key can no longer restore those secrets**: back up `keys/` with the dump, separately. Deployments without a persistent disk set `ARTEX_SECRET_KEY` diff --git a/db/config.go b/db/config.go index 5aea9a5..7efee3d 100644 --- a/db/config.go +++ b/db/config.go @@ -107,7 +107,10 @@ func scanProfile(sc interface{ Scan(...any) error }, into *string, p *LLMProfile &p.Retry.Stream.Attempts, &p.Retry.Stream.IntervalMS, &p.PriceInputPerM, &p.PriceOutputPerM, &p.PriceCacheReadPerM, &p.PriceCacheWritePerM, &p.UserAgent) - if err != nil || into != &p.APIKey { + if err != nil { + return err + } + if p.Proxy, err = openSecret(colLLMProxy, p.Proxy); err != nil || into != &p.APIKey { return err } p.APIKey, err = openSecret(colLLMKey, p.APIKey) @@ -191,12 +194,16 @@ func (d *DB) SaveProfile(p *LLMProfile) (int64, error) { if err != nil { return 0, err } + proxy, err := sealSecret(colLLMProxy, p.Proxy) + if err != nil { + return 0, err + } r := p.Retry.Clamped() if p.ID == 0 { var id int64 err := d.QueryRow(`INSERT INTO llm_profiles(name,format,base_url,proxy,model,api_key,api_key_hint,rate_per_second,rate_per_minute,context_window_k,reasoning_effort,priority,pool_exclude,thinking_type,streaming,max_tokens,max_tokens_field,session_header_key,retry_connect_attempts,retry_connect_interval_ms,retry_empty_attempts,retry_empty_interval_ms,retry_stream_attempts,retry_stream_interval_ms,user_agent) VALUES ($1,$2,NULLIF($3,''),NULLIF($4,''),$5,NULLIF($6,''),NULLIF($7,''),$8,$9,$10,$11,$12,$13,$14,$15,$16,$17,$18,$19,$20,$21,$22,$23,$24,$25) RETURNING id`, - p.Name, p.Format, p.BaseURL, p.Proxy, p.Model, key, hint, p.RatePerSecond, p.RatePerMinute, p.ContextWindowK, p.ReasoningEffort, p.Priority, p.PoolExclude, p.ThinkingType, p.Streaming, p.MaxTokens, p.MaxTokensField, p.SessionHeaderKey, + p.Name, p.Format, p.BaseURL, proxy, p.Model, key, hint, p.RatePerSecond, p.RatePerMinute, p.ContextWindowK, p.ReasoningEffort, p.Priority, p.PoolExclude, p.ThinkingType, p.Streaming, p.MaxTokens, p.MaxTokensField, p.SessionHeaderKey, r.Connect.Attempts, r.Connect.IntervalMS, r.Empty.Attempts, r.Empty.IntervalMS, r.Stream.Attempts, r.Stream.IntervalMS, p.UserAgent).Scan(&id) if err == nil { _, err = d.saveProfilePrices(id, p) @@ -205,7 +212,7 @@ VALUES ($1,$2,NULLIF($3,''),NULLIF($4,''),$5,NULLIF($6,''),NULLIF($7,''),$8,$9,$ } if p.APIKey == "" { _, err := d.Exec(`UPDATE llm_profiles SET name=$1,format=$2,base_url=NULLIF($3,''),proxy=NULLIF($4,''),model=$5,rate_per_second=$6,rate_per_minute=$7,context_window_k=$8,reasoning_effort=$9,priority=$10,pool_exclude=$11,thinking_type=$12,streaming=$13,max_tokens=$14,max_tokens_field=$15,session_header_key=$16,retry_connect_attempts=$17,retry_connect_interval_ms=$18,retry_empty_attempts=$19,retry_empty_interval_ms=$20,retry_stream_attempts=$21,retry_stream_interval_ms=$22,user_agent=$23 WHERE id=$24`, - p.Name, p.Format, p.BaseURL, p.Proxy, p.Model, p.RatePerSecond, p.RatePerMinute, p.ContextWindowK, p.ReasoningEffort, p.Priority, p.PoolExclude, p.ThinkingType, p.Streaming, p.MaxTokens, p.MaxTokensField, p.SessionHeaderKey, + p.Name, p.Format, p.BaseURL, proxy, p.Model, p.RatePerSecond, p.RatePerMinute, p.ContextWindowK, p.ReasoningEffort, p.Priority, p.PoolExclude, p.ThinkingType, p.Streaming, p.MaxTokens, p.MaxTokensField, p.SessionHeaderKey, r.Connect.Attempts, r.Connect.IntervalMS, r.Empty.Attempts, r.Empty.IntervalMS, r.Stream.Attempts, r.Stream.IntervalMS, p.UserAgent, p.ID) if err == nil { _, err = d.saveProfilePrices(p.ID, p) @@ -213,7 +220,7 @@ VALUES ($1,$2,NULLIF($3,''),NULLIF($4,''),$5,NULLIF($6,''),NULLIF($7,''),$8,$9,$ return p.ID, err } _, err = d.Exec(`UPDATE llm_profiles SET name=$1,format=$2,base_url=NULLIF($3,''),proxy=NULLIF($4,''),model=$5,api_key=$6,api_key_hint=$7,rate_per_second=$8,rate_per_minute=$9,context_window_k=$10,reasoning_effort=$11,priority=$12,pool_exclude=$13,thinking_type=$14,streaming=$15,max_tokens=$16,max_tokens_field=$17,session_header_key=$18,retry_connect_attempts=$19,retry_connect_interval_ms=$20,retry_empty_attempts=$21,retry_empty_interval_ms=$22,retry_stream_attempts=$23,retry_stream_interval_ms=$24,user_agent=$25 WHERE id=$26`, - p.Name, p.Format, p.BaseURL, p.Proxy, p.Model, key, hint, p.RatePerSecond, p.RatePerMinute, p.ContextWindowK, p.ReasoningEffort, p.Priority, p.PoolExclude, p.ThinkingType, p.Streaming, p.MaxTokens, p.MaxTokensField, p.SessionHeaderKey, + p.Name, p.Format, p.BaseURL, proxy, p.Model, key, hint, p.RatePerSecond, p.RatePerMinute, p.ContextWindowK, p.ReasoningEffort, p.Priority, p.PoolExclude, p.ThinkingType, p.Streaming, p.MaxTokens, p.MaxTokensField, p.SessionHeaderKey, r.Connect.Attempts, r.Connect.IntervalMS, r.Empty.Attempts, r.Empty.IntervalMS, r.Stream.Attempts, r.Stream.IntervalMS, p.UserAgent, p.ID) if err == nil { _, err = d.saveProfilePrices(p.ID, p) diff --git a/db/secrets.go b/db/secrets.go index 84af5f5..ba95431 100644 --- a/db/secrets.go +++ b/db/secrets.go @@ -19,7 +19,8 @@ import ( "sync/atomic" ) -// Secrets at rest: AES-256-GCM, a random nonce per value, AAD ".". +// Secrets at rest: AES-256-GCM, a random nonce per value, AAD "
." +// ("settings." for the secret settings). // TEXT stores "enc:v1:"; JSONB stores // {"__artex_enc":"enc:v1:..."} over the canonical JSON. Empty values stay empty. // A value that fails to decrypt is an error, never returned as-is. @@ -34,9 +35,32 @@ const ( type secretColumn struct { table, col string json bool + setting string // a settings row: its key; "" for a table column } -func (c secretColumn) aad() string { return c.table + "." + c.col } +func (c secretColumn) aad() string { + if c.setting != "" { + return "settings." + c.setting + } + return c.table + "." + c.col +} + +// rowFilter limits a settings entry to its own row; the key is a code-owned +// constant from secretSettingKeys, never input. +func (c secretColumn) rowFilter() string { + if c.setting != "" { + return ` AND key='` + c.setting + `'` + } + return "" +} + +// idCol names the row key: settings rows are keyed by name, the rest by id. +func (c secretColumn) idCol() string { + if c.setting != "" { + return "key" + } + return "id" +} // valueExpr is the SQL text that holds the ciphertext, if any. func (c secretColumn) valueExpr() string { @@ -47,13 +71,37 @@ func (c secretColumn) valueExpr() string { } var ( - colLLMKey = secretColumn{"llm_profiles", "api_key", false} - colMCPEnv = secretColumn{"mcp_servers", "env", true} - colCredPass = secretColumn{"task_credentials", "password", false} - colNotifConfig = secretColumn{"notification_channels", "config", true} - secretColumns = []secretColumn{colLLMKey, colMCPEnv, colCredPass, colNotifConfig} + colLLMKey = secretColumn{"llm_profiles", "api_key", false, ""} + colLLMProxy = secretColumn{"llm_profiles", "proxy", false, ""} // may carry user:pass + colMCPEnv = secretColumn{"mcp_servers", "env", true, ""} + colCredPass = secretColumn{"task_credentials", "password", false, ""} + colNotifConfig = secretColumn{"notification_channels", "config", true, ""} + secretColumns = append([]secretColumn{colLLMKey, colLLMProxy, colMCPEnv, colCredPass, colNotifConfig}, secretSettingColumns()...) ) +// secretSettingKeys are the settings stored encrypted, AAD "settings.". +// Every other setting stays in clear. +var secretSettingKeys = []string{"brave_search_api_key", "tavily_search_api_key", "web_search_proxy", "global_proxy"} + +func secretSettingColumns() []secretColumn { + out := make([]secretColumn, 0, len(secretSettingKeys)) + for _, k := range secretSettingKeys { + out = append(out, secretColumn{"settings", "value", false, k}) + } + return out +} + +// secretSetting returns the secret column of a settings key, ok=false when the +// key is stored in clear. +func secretSetting(key string) (secretColumn, bool) { + for _, c := range secretColumns { + if c.setting != "" && c.setting == key { + return c, true + } + } + return secretColumn{}, false +} + type secretBox struct{ aead cipher.AEAD } var activeSecretBox atomic.Pointer[secretBox] @@ -203,7 +251,7 @@ func (d *DB) InitSecrets(keyDir string) error { // A key other than the one that sealed the data fails here, at startup. for _, c := range secretColumns { var v string - err := conn.QueryRowContext(ctx, `SELECT `+c.valueExpr()+` FROM `+c.table+` WHERE `+c.valueExpr()+` LIKE 'enc:v1:%' LIMIT 1`).Scan(&v) + err := conn.QueryRowContext(ctx, `SELECT `+c.valueExpr()+` FROM `+c.table+` WHERE `+c.valueExpr()+` LIKE 'enc:v1:%'`+c.rowFilter()+` LIMIT 1`).Scan(&v) if err == nil { _, err = box.decrypt(c.aad(), v) } @@ -241,7 +289,7 @@ func loadSecretKey(ctx context.Context, conn *sql.Conn, keyDir string) ([]byte, } var parts []string for _, c := range secretColumns { - parts = append(parts, `(SELECT count(*) FROM `+c.table+` WHERE `+c.valueExpr()+` LIKE 'enc:v1:%')`) + parts = append(parts, `(SELECT count(*) FROM `+c.table+` WHERE `+c.valueExpr()+` LIKE 'enc:v1:%'`+c.rowFilter()+`)`) } var n int64 if err := conn.QueryRowContext(ctx, `SELECT `+strings.Join(parts, "+")).Scan(&n); err != nil { @@ -279,14 +327,14 @@ func loadSecretKey(ctx context.Context, conn *sql.Conn, keyDir string) ([]byte, func backfillSecrets(ctx context.Context, conn *sql.Conn, box *secretBox) (int, error) { total := 0 for _, c := range secretColumns { - var ids []int64 + var ids []any // int64 ids, or the settings key var olds, news []string - rows, err := conn.QueryContext(ctx, `SELECT id, `+c.col+`::text FROM `+c.table+` WHERE `+c.col+` IS NOT NULL`) + rows, err := conn.QueryContext(ctx, `SELECT `+c.idCol()+`, `+c.col+`::text FROM `+c.table+` WHERE `+c.col+` IS NOT NULL`+c.rowFilter()) if err != nil { return total, err } for rows.Next() { - var id int64 + var id any var old string if err = rows.Scan(&id, &old); err != nil { break @@ -296,7 +344,7 @@ func backfillSecrets(ctx context.Context, conn *sql.Conn, box *secretBox) (int, } nv, serr := box.seal(c, old) if serr != nil { - err = fmt.Errorf("backfill %s id=%d: %w", c.aad(), id, serr) + err = fmt.Errorf("backfill %s id=%v: %w", c.aad(), id, serr) break } if isSealed(c, nv) { // empty values stay as they are @@ -315,9 +363,9 @@ func backfillSecrets(ctx context.Context, conn *sql.Conn, box *secretBox) (int, cast = "::jsonb" } for i, id := range ids { - res, err := conn.ExecContext(ctx, `UPDATE `+c.table+` SET `+c.col+`=$1`+cast+` WHERE id=$2 AND `+c.col+`::text=$3`, news[i], id, olds[i]) + res, err := conn.ExecContext(ctx, `UPDATE `+c.table+` SET `+c.col+`=$1`+cast+` WHERE `+c.idCol()+`=$2 AND `+c.col+`::text=$3`, news[i], id, olds[i]) if err != nil { - return total, fmt.Errorf("backfill %s id=%d: %w", c.aad(), id, err) + return total, fmt.Errorf("backfill %s id=%v: %w", c.aad(), id, err) } if n, _ := res.RowsAffected(); n > 0 { total++ diff --git a/db/secrets_test.go b/db/secrets_test.go index e1da516..66d6f7f 100644 --- a/db/secrets_test.go +++ b/db/secrets_test.go @@ -5,6 +5,7 @@ import ( "encoding/base64" "encoding/hex" "encoding/json" + "fmt" "os" "path/filepath" "strings" @@ -54,7 +55,20 @@ func TestSecretBoxCrypto(t *testing.T) { type secretRows struct{ llm, mcp, cred, ch, task int64 } -var plainSecrets = []string{"sk-plain-llm", "mcp-plain-tok", "plain-pass", "hook-plain-secret"} +var plainSecrets = []string{"sk-plain-llm", "mcp-plain-tok", "plain-pass", "hook-plain-secret", "llmproxy-plain-pass", + "brave-plain", "tavily-plain", "wsproxy-plain-pass", "gproxy-plain-pass"} + +// secretSettingValues are the four secret settings written by the fixture. +var secretSettingValues = map[string]string{ + "brave_search_api_key": "brave-plain", + "tavily_search_api_key": "tavily-plain", + "web_search_proxy": "socks5://u:wsproxy-plain-pass@ws.example:1080", + "global_proxy": "http://u:gproxy-plain-pass@gp.example:3128", +} + +const llmProxyPlain = "http://u:llmproxy-plain-pass@llm-proxy.example:3128" + +const deleteSecretSettings = `DELETE FROM settings WHERE key IN ('brave_search_api_key','tavily_search_api_key','web_search_proxy','global_proxy')` // secretFixture writes one secret to each location, through the db API or, for // a legacy database, as raw plaintext SQL. @@ -70,6 +84,7 @@ func secretFixture(t *testing.T, d *DB, legacy bool) (r secretRows) { d.Exec(`DELETE FROM llm_profiles WHERE id=$1`, r.llm) d.Exec(`DELETE FROM mcp_servers WHERE id=$1`, r.mcp) d.Exec(`DELETE FROM notification_channels WHERE id=$1`, r.ch) + d.Exec(deleteSecretSettings) }) env, cfg := `{"Authorization":"mcp-plain-tok"}`, `{"url":"https://h.example","headers":{"X":"hook-plain-secret"}}` if legacy { @@ -78,13 +93,23 @@ func secretFixture(t *testing.T, d *DB, legacy bool) (r secretRows) { t.Fatal(err) } } - ins(&r.llm, `INSERT INTO llm_profiles(name,format,model,api_key) VALUES ('t-sec','openai','m','sk-plain-llm') RETURNING id`) + ins(&r.llm, `INSERT INTO llm_profiles(name,format,model,api_key,proxy) VALUES ('t-sec','openai','m','sk-plain-llm',$1) RETURNING id`, llmProxyPlain) + for k, v := range secretSettingValues { + if _, err := d.Exec(`INSERT INTO settings(key,value) VALUES ($1,$2)`, k, v); err != nil { + t.Fatal(err) + } + } ins(&r.mcp, `INSERT INTO mcp_servers(name,transport,env) VALUES ('t-sec','http',$1) RETURNING id`, env) ins(&r.cred, `INSERT INTO task_credentials(task_id,domain,username,password) VALUES ($1,'x.example','u','plain-pass') RETURNING id`, r.task) ins(&r.ch, `INSERT INTO notification_channels(name,kind,config) VALUES ('t-sec','webhook',$1) RETURNING id`, cfg) return r } - r.llm, err = d.SaveProfile(&LLMProfile{Name: "t-sec", Format: "openai", Model: "m", APIKey: "sk-plain-llm"}) + r.llm, err = d.SaveProfile(&LLMProfile{Name: "t-sec", Format: "openai", Model: "m", APIKey: "sk-plain-llm", Proxy: llmProxyPlain}) + for k, v := range secretSettingValues { + if err == nil { + err = d.SetSetting(k, v) + } + } if err == nil { r.mcp, err = d.SaveMCP(&MCPServer{Name: "t-sec", Transport: "http", Env: json.RawMessage(env)}) } @@ -102,19 +127,28 @@ func secretFixture(t *testing.T, d *DB, legacy bool) (r secretRows) { return r } -// rawSecrets reads the four columns as stored in PostgreSQL. +// rawSecrets reads the secret columns and settings as stored in PostgreSQL. func rawSecrets(t *testing.T, d *DB, r secretRows) string { t.Helper() - var a, b, c, e string + var a, b, c, e, px string if err := d.QueryRow(`SELECT (SELECT api_key FROM llm_profiles WHERE id=$1), (SELECT env::text FROM mcp_servers WHERE id=$2), -(SELECT password FROM task_credentials WHERE id=$3), (SELECT config::text FROM notification_channels WHERE id=$4)`, - r.llm, r.mcp, r.cred, r.ch).Scan(&a, &b, &c, &e); err != nil { +(SELECT password FROM task_credentials WHERE id=$3), (SELECT config::text FROM notification_channels WHERE id=$4), +(SELECT proxy FROM llm_profiles WHERE id=$1)`, + r.llm, r.mcp, r.cred, r.ch).Scan(&a, &b, &c, &e, &px); err != nil { t.Fatal(err) } - if !isEncrypted(a) || !isSealed(colMCPEnv, b) || !isEncrypted(c) || !isSealed(colNotifConfig, e) { + if !isEncrypted(a) || !isSealed(colMCPEnv, b) || !isEncrypted(c) || !isSealed(colNotifConfig, e) || !isEncrypted(px) { t.Fatal("a stored secret is not encrypted") } - all := strings.Join([]string{a, b, c, e}, "|") + vals := []string{a, b, c, e, px} + for _, k := range secretSettingKeys { + var v string + if err := d.QueryRow(`SELECT value FROM settings WHERE key=$1`, k).Scan(&v); err != nil || !isEncrypted(v) { + t.Fatalf("setting %s not encrypted at rest (%v)", k, err) + } + vals = append(vals, v) + } + all := strings.Join(vals, "|") for _, p := range plainSecrets { if strings.Contains(all, p) { t.Fatalf("plaintext %q found at rest", p) @@ -128,7 +162,20 @@ func assertPlainReads(t *testing.T, d *DB, r secretRows) { t.Helper() got := "" if p, err := d.ProfileByID(r.llm); err == nil && p != nil { - got += p.APIKey + got += p.APIKey + p.Proxy + } + if ps, err := d.ListProfiles(); err == nil { + for _, p := range ps { + if p.ID == r.llm && p.Proxy != llmProxyPlain { + t.Fatalf("ListProfiles proxy %q not decrypted", p.Proxy) + } + } + } + for k, want := range secretSettingValues { + if v, ok, err := d.GetSetting(k); err != nil || !ok || v != want { + t.Fatalf("GetSetting(%s) = %q %v %v", k, v, ok, err) + } + got += want } ms, _ := d.ListMCP() for _, m := range ms { @@ -194,4 +241,87 @@ func TestSecretsBackfillLegacyAndKeyChecks(t *testing.T) { t.Fatalf("want wrong-key refusal, got %v", err) } assertPlainReads(t, d, r) // the active key is unchanged + + // The no-key refusal counts the four encrypted settings. + t.Setenv(secretKeyEnv, "") + withSettings := encryptedCount(t, d, dir) + if _, err := d.Exec(deleteSecretSettings); err != nil { + t.Fatal(err) + } + if n := encryptedCount(t, d, dir); withSettings-n != len(secretSettingKeys) { + t.Fatalf("no-key refusal counted %d then %d encrypted values; the settings must count", withSettings, n) + } + + // A setting sealed under another key fails the startup key check. + other, _ := newSecretBox([]byte("ffffffffffffffffffffffffffffffff")) + v, _ := other.encrypt("settings.global_proxy", "http://other.example:3128") + if _, err := d.Exec(`INSERT INTO settings(key,value) VALUES ('global_proxy',$1)`, v); err != nil { + t.Fatal(err) + } + t.Setenv(secretKeyEnv, base64.StdEncoding.EncodeToString(testSecretKey)) + if err := d.InitSecrets(t.TempDir()); err == nil || !strings.Contains(err.Error(), "settings.global_proxy") { + t.Fatalf("want a key-check failure on settings.global_proxy, got %v", err) + } + if _, err := d.Exec(deleteSecretSettings); err != nil { + t.Fatal(err) + } +} + +// encryptedCount reads the encrypted-value count from the no-key refusal. +func encryptedCount(t *testing.T, d *DB, dir string) int { + t.Helper() + err := d.InitSecrets(dir) + var n int + if err == nil { + t.Fatal("want a no-key refusal") + } + if _, serr := fmt.Sscanf(err.Error(), "the database holds %d encrypted", &n); serr != nil { + t.Fatalf("refusal %q: %v", err, serr) + } + return n +} + +// Secret settings: empty stays empty, InsertSettingIfAbsent encrypts, other +// settings stay in clear; an empty LLM proxy stays empty. +func TestSecretSettingsEmptyAndClear(t *testing.T) { + d, err := Open(testDSN(t)) + if err != nil { + t.Skipf("postgres unavailable (%v) — skipping", err) + } + t.Cleanup(func() { + d.Exec(deleteSecretSettings) + d.Exec(`DELETE FROM settings WHERE key='t_sec_clear'`) + d.Exec(`DELETE FROM llm_profiles WHERE name='t-sec-noproxy'`) + d.Close() + }) + raw := func(k string) string { + var v string + if err := d.QueryRow(`SELECT value FROM settings WHERE key=$1`, k).Scan(&v); err != nil { + t.Fatal(err) + } + return v + } + if err := d.SetSetting("global_proxy", ""); err != nil || raw("global_proxy") != "" { + t.Fatalf("empty secret setting must stay empty (%v)", err) + } + if ok, err := d.InsertSettingIfAbsent("tavily_search_api_key", "tavily-insert"); err != nil || !ok || !isEncrypted(raw("tavily_search_api_key")) { + t.Fatalf("InsertSettingIfAbsent must encrypt a secret setting (%v %v)", ok, err) + } + if v, _, err := d.GetSetting("tavily_search_api_key"); err != nil || v != "tavily-insert" { + t.Fatalf("read back %q %v", v, err) + } + if err := d.SetSetting("t_sec_clear", "visible"); err != nil || raw("t_sec_clear") != "visible" { + t.Fatalf("a non-secret setting must stay in clear (%v)", err) + } + id, err := d.SaveProfile(&LLMProfile{Name: "t-sec-noproxy", Format: "openai", Model: "m", APIKey: "k"}) + if err != nil { + t.Fatal(err) + } + var px *string + if err := d.QueryRow(`SELECT proxy FROM llm_profiles WHERE id=$1`, id).Scan(&px); err != nil || px != nil { + t.Fatalf("empty proxy stored as %v (%v), want NULL", px, err) + } + if p, err := d.ProfileByID(id); err != nil || p.Proxy != "" { + t.Fatalf("empty proxy read back as %q (%v)", p.Proxy, err) + } } diff --git a/db/settings.go b/db/settings.go index 82e0d79..7b3a307 100644 --- a/db/settings.go +++ b/db/settings.go @@ -5,7 +5,8 @@ import "database/sql" // Settings is a tiny key-value store for global app config the UI toggles at // runtime (e.g. traffic_capture). Missing keys fall back to caller defaults. -// GetSetting returns the stored value and ok=false when the key is unset. +// GetSetting returns the stored value and ok=false when the key is unset. A +// secret setting (secretSettingKeys) is decrypted. func (d *DB) GetSetting(key string) (value string, ok bool, err error) { err = d.QueryRow(`SELECT value FROM settings WHERE key=$1`, key).Scan(&value) if err == sql.ErrNoRows { @@ -14,12 +15,29 @@ func (d *DB) GetSetting(key string) (value string, ok bool, err error) { if err != nil { return "", false, err } + if c, secret := secretSetting(key); secret { + if value, err = openSecret(c, value); err != nil { + return "", false, err + } + } return value, true, nil } +// sealSetting encrypts the value of a secret setting; other values pass as is. +func sealSetting(key, value string) (string, error) { + if c, secret := secretSetting(key); secret { + return sealSecret(c, value) + } + return value, nil +} + // SetSetting upserts a setting value. func (d *DB) SetSetting(key, value string) error { - _, err := d.Exec(` + value, err := sealSetting(key, value) + if err != nil { + return err + } + _, err = d.Exec(` INSERT INTO settings(key, value) VALUES ($1, $2) ON CONFLICT (key) DO UPDATE SET value = EXCLUDED.value, updated_at = now()`, key, value) return err @@ -32,6 +50,9 @@ ON CONFLICT (key) DO UPDATE SET value = EXCLUDED.value, updated_at = now()`, key // fast-fail path — on a read error or a concurrent race, the existing value will not be // silently overwritten by ON CONFLICT DO UPDATE. func (d *DB) InsertSettingIfAbsent(key, value string) (inserted bool, err error) { + if value, err = sealSetting(key, value); err != nil { + return false, err + } res, err := d.Exec(` INSERT INTO settings(key, value) VALUES ($1, $2) ON CONFLICT (key) DO NOTHING`, key, value) diff --git a/server/dto.go b/server/dto.go index 349eeb7..2a9963d 100644 --- a/server/dto.go +++ b/server/dto.go @@ -603,7 +603,8 @@ type LLMProfileDTO struct { Name string `json:"name"` Format string `json:"format"` BaseURL string `json:"base_url,omitempty"` - Proxy string `json:"proxy,omitempty"` + Proxy string `json:"proxy,omitempty"` // masked: the placeholder when set + ProxySet bool `json:"proxy_set"` Model string `json:"model"` APIKeyHint string `json:"api_key_hint,omitempty"` RatePerSecond float64 `json:"rate_per_second"` @@ -641,7 +642,8 @@ func llmProfileDTO(p *db.LLMProfile) LLMProfileDTO { Name: p.Name, Format: p.Format, BaseURL: p.BaseURL, - Proxy: p.Proxy, + Proxy: maskedSecret(p.Proxy), + ProxySet: p.Proxy != "", Model: p.Model, APIKeyHint: p.APIKeyHint, RatePerSecond: p.RatePerSecond, diff --git a/server/secrets_api_test.go b/server/secrets_api_test.go index 1266654..2422cd9 100644 --- a/server/secrets_api_test.go +++ b/server/secrets_api_test.go @@ -144,3 +144,118 @@ func TestMCPMaskedEnvDestinationGuard(t *testing.T) { t.Fatal("explicit env with a new url was not saved") } } + +// Secret settings and the LLM profile proxy: stored encrypted, never returned +// in clear; on write the placeholder keeps, an explicit value replaces, "" +// clears. +func TestProxySecretsAPI(t *testing.T) { + s, _, request := trafficEvidenceServer(t) + pg := s.m.PG() + t.Cleanup(func() { + pg.Exec(`DELETE FROM settings WHERE key IN ('brave_search_api_key','tavily_search_api_key','web_search_proxy','global_proxy')`) + pg.Exec(`DELETE FROM llm_profiles WHERE name='t-proxy'`) + }) + secrets := []string{"brave-api-0007", "tavily-api-0008", "wsp-pass-0009", "gp-pass-0010", "llmp-pass-0011"} + put := func(body string) { + t.Helper() + resp := request("PUT", "/api/settings", body) + for _, sec := range secrets { + if resp.Code != 200 || strings.Contains(resp.Body.String(), sec) { + t.Fatalf("PUT /api/settings %s: HTTP %d or a secret returned in clear", body, resp.Code) + } + } + } + setting := func(k string) string { + t.Helper() + var raw string + if err := pg.QueryRow(`SELECT value FROM settings WHERE key=$1`, k).Scan(&raw); err != nil { + t.Fatal(err) + } + v, _, err := pg.GetSetting(k) + if err != nil || (raw != "" && !strings.HasPrefix(raw, "enc:v1:")) { + t.Fatalf("setting %s stored as %q (%v)", k, raw, err) + } + return v + } + put(`{"brave_search_api_key":"brave-api-0007","tavily_search_api_key":"tavily-api-0008", +"web_search_proxy":"socks5://u:wsp-pass-0009@ws.example:1080","global_proxy":"http://u:gp-pass-0010@gp.example:3128"}`) + var got map[string]any + resp := request("GET", "/api/settings", "") + _ = json.Unmarshal(resp.Body.Bytes(), &got) + for _, sec := range secrets { + if strings.Contains(resp.Body.String(), sec) { + t.Fatal("GET /api/settings returned a secret in clear") + } + } + if got["global_proxy"] != "__masked__" || got["global_proxy_set"] != true || got["web_search_proxy"] != "__masked__" || got["web_search_proxy_set"] != true { + t.Fatalf("proxies not masked with a presence flag: %v", got) + } + + put(`{"web_search_proxy":"__masked__","global_proxy":"__masked__","brave_search_api_key":"__masked__"}`) + if setting("web_search_proxy") != "socks5://u:wsp-pass-0009@ws.example:1080" || setting("global_proxy") != "http://u:gp-pass-0010@gp.example:3128" || + setting("brave_search_api_key") != "brave-api-0007" || setting("tavily_search_api_key") != "tavily-api-0008" { + t.Fatal("a masked placeholder replaced a stored secret") + } + put(`{"global_proxy":"http://gp2.example:3128","web_search_proxy":""}`) + if setting("global_proxy") != "http://gp2.example:3128" || setting("web_search_proxy") != "" { + t.Fatal("explicit value must replace and \"\" must clear") + } + _ = json.Unmarshal(request("GET", "/api/settings", "").Body.Bytes(), &got) + if got["web_search_proxy"] != "" || got["web_search_proxy_set"] != false { + t.Fatalf("cleared proxy shown as %v / %v", got["web_search_proxy"], got["web_search_proxy_set"]) + } + + // LLM profile proxy. + id, err := pg.SaveProfile(&db.LLMProfile{Name: "t-proxy", Format: "openai", Model: "m", APIKey: "k", Proxy: "http://u:llmp-pass-0011@llm.example:3128"}) + if err != nil { + t.Fatal(err) + } + proxy := func() string { + var raw *string + if err := pg.QueryRow(`SELECT proxy FROM llm_profiles WHERE id=$1`, id).Scan(&raw); err != nil { + t.Fatal(err) + } + if raw != nil && !strings.HasPrefix(*raw, "enc:v1:") { + t.Fatalf("profile proxy stored in clear: %q", *raw) + } + p, err := pg.ProfileByID(id) + if err != nil { + t.Fatal(err) + } + return p.Proxy + } + for _, path := range []string{"/api/llm/profiles", "/api/llm"} { + if resp := request("GET", path, ""); resp.Code != 200 || strings.Contains(resp.Body.String(), "llmp-pass-0011") { + t.Fatalf("GET %s: HTTP %d or the proxy returned in clear", path, resp.Code) + } + } + var list struct{ Profiles []LLMProfileDTO } + _ = json.Unmarshal(request("GET", "/api/llm/profiles", "").Body.Bytes(), &list) + for _, p := range list.Profiles { + if p.Name == "t-proxy" && (p.Proxy != "__masked__" || !p.ProxySet) { + t.Fatalf("profile proxy not masked: %+v", p) + } + } + save := func(proxyJSON string, want int) { + t.Helper() + body := `{"id":` + strconv.FormatInt(id, 10) + `,"name":"t-proxy","format":"openai","model":"m","proxy":` + proxyJSON + `}` + if resp := request("POST", "/api/llm/profiles", body); resp.Code != want { + t.Fatalf("save proxy %s: want HTTP %d, got %d %s", proxyJSON, want, resp.Code, resp.Body.String()) + } + } + save(`"__masked__"`, 200) + if proxy() != "http://u:llmp-pass-0011@llm.example:3128" { + t.Fatal("the placeholder replaced the stored profile proxy") + } + save(`"socks5://llm2.example:1080"`, 200) + if proxy() != "socks5://llm2.example:1080" { + t.Fatal("explicit proxy not saved") + } + save(`""`, 200) + if proxy() != "" { + t.Fatal("empty proxy must clear") + } + if resp := request("POST", "/api/llm/profiles", `{"name":"t-proxy","format":"openai","model":"m","proxy":"__masked__"}`); resp.Code != 400 { + t.Fatalf("a placeholder on a new profile must be refused, got HTTP %d", resp.Code) + } +} diff --git a/server/server.go b/server/server.go index e857d2a..4f017b1 100644 --- a/server/server.go +++ b/server/server.go @@ -1355,7 +1355,8 @@ func (s *Server) getLLM(w http.ResponseWriter, r *http.Request) { "provider": s.llmCfg.Provider(), "model": s.llmCfg.Model, "base_url": s.llmCfg.BaseURL, - "proxy": s.llmCfg.Proxy, + "proxy": maskedSecret(s.llmCfg.Proxy), + "proxy_set": s.llmCfg.Proxy != "", "key_set": s.llmCfg.APIKey != "", "rate_per_second": s.llmCfg.RatePerSecond, "rate_per_minute": s.llmCfg.RatePerMinute, @@ -1383,6 +1384,11 @@ func (s *Server) setLLM(w http.ResponseWriter, r *http.Request) { writeErr(w, 400, err.Error()) return } + if isMaskedSecret(req.Proxy) { // getLLM returns the proxy masked: keep the current one + s.cfgMu.Lock() + req.Proxy = s.llmCfg.Proxy + s.cfgMu.Unlock() + } cfg := agent.ConfigFrom(req.Provider, req.Model, req.BaseURL, req.APIKey, req.Proxy) cfg.RatePerSecond, cfg.RatePerMinute = req.RatePerSecond, req.RatePerMinute cfg.ThinkingType = req.ThinkingType @@ -1455,6 +1461,14 @@ func (s *Server) testLLM(w http.ResponseWriter, r *http.Request) { stored = p } } + if isMaskedSecret(req.Proxy) { // the form echoes the masked proxy: test with the stored one + req.Proxy = "" + if stored == nil { + s.cfgMu.Lock() + req.Proxy = s.llmCfg.Proxy + s.cfgMu.Unlock() + } + } if stored != nil { if strings.TrimSpace(req.Provider) == "" { req.Provider = stored.Format @@ -3487,8 +3501,10 @@ func (s *Server) settingsPayload() map[string]any { "web_search_backend": backend, "brave_key_set": strings.TrimSpace(braveKey) != "", "tavily_key_set": strings.TrimSpace(tavilyKey) != "", - "web_search_proxy": proxy, // dedicated egress proxy (http/https/socks5), empty = direct - "global_proxy": s.m.GlobalProxy(), // global egress proxy (http/https/socks5), all target traffic goes through it, empty = direct + "web_search_proxy": maskedSecret(proxy), // proxies may carry user:pass: masked when set, "" = direct + "web_search_proxy_set": proxy != "", + "global_proxy": maskedSecret(s.m.GlobalProxy()), // global egress proxy, masked when set, "" = direct + "global_proxy_set": s.m.GlobalProxy() != "", "python_interpreter": strings.TrimSpace(pyStored), // user/auto-set value (empty = use runtime detection) "workers": s.m.Workers(), // number of concurrent worker agents (default 3); applies to tasks started afterwards "task_concurrency_enabled": concOn, // task concurrency cap switch (default off) @@ -3591,6 +3607,9 @@ func (s *Server) putSettings(w http.ResponseWriter, r *http.Request) { writeErr(w, 400, err.Error()) return } + // A masked placeholder echoed back keeps the stored secret, like null. + req.BraveKey, req.TavilyKey = keepIfMasked(req.BraveKey), keepIfMasked(req.TavilyKey) + req.WebSearchProxy, req.GlobalProxy = keepIfMasked(req.WebSearchProxy), keepIfMasked(req.GlobalProxy) if req.ConstraintsInjectPlanner != nil { if err := s.m.pg.SetBool(settingConstraintsInjectPlanner, *req.ConstraintsInjectPlanner); err != nil { writeErr(w, 500, err.Error()) @@ -3788,13 +3807,17 @@ func (s *Server) testWebSearch(w http.ResponseWriter, r *http.Request) { writeErr(w, 400, err.Error()) return } - _, backend, storedBraveKey, storedTavilyKey, _ := s.m.WebSearch() + _, backend, storedBraveKey, storedTavilyKey, storedProxy := s.m.WebSearch() if strings.TrimSpace(req.Backend) != "" { backend = req.Backend } // Proxy is taken from the form as-is (empty = direct), so testing reflects exactly // what's shown — including an intentional "clear proxy to test direct" before saving. + // The masked placeholder the form shows for a saved proxy means the saved one. proxy := strings.TrimSpace(req.Proxy) + if isMaskedSecret(proxy) { + proxy = storedProxy + } // API keys are secrets the form omits when already saved, so fall back to stored. braveKey := storedBraveKey if strings.TrimSpace(req.BraveKey) != "" { diff --git a/server/server_mgmt.go b/server/server_mgmt.go index c236627..835e655 100644 --- a/server/server_mgmt.go +++ b/server/server_mgmt.go @@ -864,6 +864,26 @@ func (s *Server) pgListMCP(w http.ResponseWriter, r *http.Request) { writeJSON(w, 200, map[string]any{"servers": ms}) } +// maskedSecret is what a response shows for a stored secret such as a proxy +// URL: the whole value is replaced by the placeholder, "" stays "". +func maskedSecret(v string) string { + if v == "" { + return "" + } + return notify.MaskedPrefix +} + +// isMaskedSecret reports a placeholder sent back on write: keep the stored value. +func isMaskedSecret(v string) bool { return notify.IsMasked(strings.TrimSpace(v)) } + +// keepIfMasked turns a placeholder in an optional field into nil (keep). +func keepIfMasked(v *string) *string { + if v != nil && isMaskedSecret(*v) { + return nil + } + return v +} + // maskMCPEnv replaces every non-empty env value (API keys, auth headers) with // notify.MaskedPrefix so the API never returns it in clear. func maskMCPEnv(raw json.RawMessage) json.RawMessage { @@ -1949,6 +1969,16 @@ func (s *Server) pgSaveProfile(w http.ResponseWriter, r *http.Request) { p := body.LLMProfile p.APIKey = body.APIKey p.Streaming = body.Streaming == nil || *body.Streaming + // The list returns the proxy masked: the placeholder sent back keeps the + // stored proxy and is never stored itself. + if isMaskedSecret(p.Proxy) { + stored, err := pg.ProfileByID(p.ID) + if p.ID == 0 || err != nil || stored == nil { + writeErr(w, 400, "proxy is a masked placeholder but no stored proxy to keep; enter the proxy or clear it") + return + } + p.Proxy = stored.Proxy + } // Output cap: negative values are meaningless, clamp to zero (= do not send the // field). The field-name switch only matters for Chat Completions — anthropic and // openai-responses hard-code their field names; storing it would only mislead later readers, so it is cleared for any non-openai format. Unknown values are cleared too, to avoid throwing DB CHECK errors at the user. @@ -2150,6 +2180,14 @@ func (s *Server) pgListModels(w http.ResponseWriter, r *http.Request) { } } } + if isMaskedSecret(req.Proxy) { // the form echoes the masked proxy: use the stored one + req.Proxy = "" + if req.ProfileID != nil { + if p, err := s.m.pg.ProfileByID(*req.ProfileID); err == nil && p != nil { + req.Proxy = p.Proxy + } + } + } provider := strings.TrimSpace(req.Provider) // Ollama serves its model list keyless on the local server: no key gate. if apiKey == "" && provider != "ollama" {