From 07cc87c1a3f62d9236488c46ae358e25ecffc140 Mon Sep 17 00:00:00 2001 From: BaaRai <130491490+BaaRai@users.noreply.github.com> Date: Fri, 13 Nov 2026 14:42:56 +0100 Subject: [PATCH 1/2] Encrypt stored secrets at rest in PostgreSQL Seal the four credential columns with AES-256-GCM (random nonce per value, AAD "."): llm_profiles.api_key and task_credentials.password store "enc:v1:"; mcp_servers.env and notification_channels.config store {"__artex_enc":"enc:v1:..."} over the canonical JSON. The db layer encrypts on write and decrypts on read, so agent tools (LLM provider, MCP, notifier, test credentials) get plaintext server-side. Decryption failure is an error, never a plaintext fallback. Key: ARTEX_SECRET_KEY (32 bytes, base64 or hex) or keys/secret.key next to jwt.key, generated with crypto/rand at first start (0600). Startup refuses when encrypted values exist and no key is available, and when the key does not decrypt them. An idempotent backfill under the schema advisory lock encrypts legacy plaintext rows and logs only a count. API: MCP env is masked on read and masked placeholders on write keep the stored value; task credential passwords are masked server-side and a masked password on write is refused. ARTEX_SECRET_KEY is scrubbed from the process environment with the other startup secrets. --- .env.example | 4 + .gitignore | 1 + README.md | 8 + cmd/artex/main.go | 12 +- config/config.go | 13 ++ db/config.go | 30 ++- db/notification.go | 18 +- db/notification_delivery.go | 5 + db/schema.sql | 2 +- db/secrets.go | 328 ++++++++++++++++++++++++++++++++ db/secrets_test.go | 197 +++++++++++++++++++ db/task_credentials.go | 15 +- db/testmain_test.go | 2 + docker-compose.yml | 2 + server/env_scrub.go | 4 +- server/env_scrub_test.go | 1 + server/manager.go | 6 + server/secrets_api_test.go | 83 ++++++++ server/server_mgmt.go | 53 ++++++ server/task_credentials.go | 15 +- server/task_credentials_test.go | 2 +- server/testmain_test.go | 2 + 22 files changed, 774 insertions(+), 29 deletions(-) create mode 100644 db/secrets.go create mode 100644 db/secrets_test.go create mode 100644 server/secrets_api_test.go diff --git a/.env.example b/.env.example index 807b3f3..c7b4007 100644 --- a/.env.example +++ b/.env.example @@ -6,6 +6,10 @@ ARTEX_BIND_ADDR=127.0.0.1 # Reverse proxies (comma-separated CIDRs) whose X-Forwarded-For the login rate # limit trusts. Empty: the header is ignored. See "Reverse proxy deployment" in README.md. ARTEX_TRUSTED_PROXIES= +# Optional key for the secrets encrypted in the database (32 bytes, base64 or hex, +# e.g. `openssl rand -base64 32`). Empty: keys/secret.key, generated at first start. +# Set it when ./keys is not persistent. Losing the key loses the stored secrets. +# ARTEX_SECRET_KEY= POSTGRES_USER=artex POSTGRES_PASSWORD=change-me-please POSTGRES_DB=artex diff --git a/.gitignore b/.gitignore index 8cca111..aeb1fc4 100644 --- a/.gitignore +++ b/.gitignore @@ -47,6 +47,7 @@ Thumbs.db # JWT signing key (generated at runtime, stored in the project root, never committed) /jwt.key +/secret.key /keys/ # local environment generated by Docker deployment (contains passwords/keys, not committed) diff --git a/README.md b/README.md index 0206942..ea8ac8e 100644 --- a/README.md +++ b/README.md @@ -181,6 +181,14 @@ 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` +(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` +(32 bytes, base64 or hex, e.g. `openssl rand -base64 32`) instead. ARTEX refuses +to start when the database holds encrypted secrets and no key is available. + Daily cron example (03:20, operator crontab): ``` diff --git a/cmd/artex/main.go b/cmd/artex/main.go index f2f0b6b..ba2a84a 100644 --- a/cmd/artex/main.go +++ b/cmd/artex/main.go @@ -13,7 +13,6 @@ import ( "os/signal" "path/filepath" "runtime" - "strings" "syscall" "time" @@ -98,15 +97,8 @@ func run() int { skillDir = abs } log.Printf("[config] skill directory: %s", skillDir) - // JWT signing key directory: ARTEX_JWT_KEY_DIR (the Docker image points it at - // the /app/keys volume) or the base dir. Never the browsable data dir. - keyDir := strings.TrimSpace(os.Getenv("ARTEX_JWT_KEY_DIR")) - if keyDir == "" { - keyDir = config.BaseDir() - } - if abs, err := filepath.Abs(keyDir); err == nil { - keyDir = abs - } + // JWT signing key directory (also holds secret.key, see server.NewManager). + keyDir := config.KeyDir() log.Printf("[config] JWT key directory: %s", keyDir) // Proxies whose X-Forwarded-For the login rate limit reads; empty trusts none. trusted, err := server.ParseTrustedProxies(os.Getenv("ARTEX_TRUSTED_PROXIES")) diff --git a/config/config.go b/config/config.go index e42487e..b966429 100644 --- a/config/config.go +++ b/config/config.go @@ -54,6 +54,19 @@ func BaseDir() string { return dir } +// KeyDir holds jwt.key and secret.key: ARTEX_JWT_KEY_DIR (the Docker image points +// it at the /app/keys volume) or the base dir. Never the browsable data dir. +func KeyDir() string { + dir := strings.TrimSpace(os.Getenv("ARTEX_JWT_KEY_DIR")) + if dir == "" { + dir = BaseDir() + } + if abs, err := filepath.Abs(dir); err == nil { + dir = abs + } + return dir +} + // isGoRunDir reports whether dir is where `go run` parked its executable: under // the system temp dir (cache miss), or anywhere inside a Go build cache // (…/go-build/…, the cache-hit case — NOT under os.TempDir(), which is why the diff --git a/db/config.go b/db/config.go index 6c779cc..5aea9a5 100644 --- a/db/config.go +++ b/db/config.go @@ -99,7 +99,7 @@ const profileColsKey = `id,name,format,COALESCE(base_url,''),COALESCE(proxy,''), // scanProfile reads one row in the profileCols / profileColsKey column order. The // 7th column lands in APIKeyHint or APIKey depending on which list the caller used. func scanProfile(sc interface{ Scan(...any) error }, into *string, p *LLMProfile) error { - return sc.Scan(&p.ID, &p.Name, &p.Format, &p.BaseURL, &p.Proxy, &p.Model, into, + err := sc.Scan(&p.ID, &p.Name, &p.Format, &p.BaseURL, &p.Proxy, &p.Model, into, &p.RatePerSecond, &p.RatePerMinute, &p.ContextWindowK, &p.ReasoningEffort, &p.IsDefault, &p.Priority, &p.PoolExclude, &p.ThinkingType, &p.Streaming, &p.MaxTokens, &p.MaxTokensField, &p.SessionHeaderKey, &p.Retry.Connect.Attempts, &p.Retry.Connect.IntervalMS, @@ -107,6 +107,11 @@ 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 { + return err + } + p.APIKey, err = openSecret(colLLMKey, p.APIKey) + return err } func (d *DB) ListProfiles() ([]*LLMProfile, error) { @@ -182,12 +187,16 @@ func (d *DB) SaveProfile(p *LLMProfile) (int64, error) { if len(p.APIKey) >= 4 { hint = "…" + p.APIKey[len(p.APIKey)-4:] } + key, err := sealSecret(colLLMKey, p.APIKey) + 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, p.APIKey, 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, 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, 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) @@ -203,8 +212,8 @@ 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, p.APIKey, hint, p.RatePerSecond, p.RatePerMinute, p.ContextWindowK, p.ReasoningEffort, p.Priority, p.PoolExclude, p.ThinkingType, p.Streaming, p.MaxTokens, p.MaxTokensField, p.SessionHeaderKey, + _, 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, 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) @@ -875,7 +884,12 @@ func (d *DB) ListMCP() ([]*MCPServer, error) { rows.Close() return nil, err } - m.Args, m.Env = json.RawMessage(args), json.RawMessage(env) + plainEnv, err := openSecret(colMCPEnv, string(env)) + if err != nil { + rows.Close() + return nil, err + } + m.Args, m.Env = json.RawMessage(args), json.RawMessage(plainEnv) out = append(out, &m) } if err := rows.Err(); err != nil { @@ -970,6 +984,10 @@ func (d *DB) SaveMCP(m *MCPServer) (int64, error) { if env == "" { env = "{}" } + env, err := sealSecret(colMCPEnv, env) + if err != nil { + return 0, err + } timeout := m.TimeoutSeconds if timeout <= 0 { timeout = 120 @@ -980,7 +998,7 @@ func (d *DB) SaveMCP(m *MCPServer) (int64, error) { m.Name, m.Transport, m.Command, args, env, m.URL, m.Enabled, m.Insecure, timeout).Scan(&id) return id, err } - _, err := d.Exec(`UPDATE mcp_servers SET name=$1,transport=$2,command=NULLIF($3,''),args=$4,env=$5,url=NULLIF($6,''),enabled=$7,insecure=$8,timeout_seconds=$9 WHERE id=$10`, + _, err = d.Exec(`UPDATE mcp_servers SET name=$1,transport=$2,command=NULLIF($3,''),args=$4,env=$5,url=NULLIF($6,''),enabled=$7,insecure=$8,timeout_seconds=$9 WHERE id=$10`, m.Name, m.Transport, m.Command, args, env, m.URL, m.Enabled, m.Insecure, timeout, m.ID) return m.ID, err } diff --git a/db/notification.go b/db/notification.go index 2422407..1a386b5 100644 --- a/db/notification.go +++ b/db/notification.go @@ -88,6 +88,11 @@ func scanNotificationChannel(sc interface{ Scan(...any) error }) (*NotificationC if err := sc.Scan(&c.ID, &c.Name, &c.Kind, &enabled, &c.Config, &c.Mode, &c.Filter, &c.RatePerMin, &c.CreatedAt, &c.UpdatedAt); err != nil { return nil, err } + cfg, err := openSecret(colNotifConfig, string(c.Config)) + if err != nil { + return nil, err + } + c.Config = json.RawMessage(cfg) c.Enabled = &enabled return &c, nil } @@ -152,19 +157,23 @@ func (d *DB) SaveNotificationChannel(ctx context.Context, c *NotificationChannel if c.Filter == nil { c.Filter = json.RawMessage(`{}`) } + cfg, err := sealSecret(colNotifConfig, string(c.Config)) + if err != nil { + return 0, err + } enabled := c.IsEnabled() if c.ID == 0 { var id int64 err := d.QueryRowContext(ctx, `INSERT INTO notification_channels(name,kind,enabled,config,mode,filter,rate_per_min) VALUES($1,$2,$3,$4,$5,$6,$7) RETURNING id`, - c.Name, c.Kind, enabled, string(c.Config), c.Mode, string(c.Filter), c.RatePerMin).Scan(&id) + c.Name, c.Kind, enabled, string(cfg), c.Mode, string(c.Filter), c.RatePerMin).Scan(&id) return id, err } res, err := d.ExecContext(ctx, `UPDATE notification_channels SET name=$2, kind=$3, enabled=$4, config=$5, mode=$6, filter=$7, rate_per_min=$8 WHERE id=$1`, - c.ID, c.Name, c.Kind, enabled, string(c.Config), c.Mode, string(c.Filter), c.RatePerMin) + c.ID, c.Name, c.Kind, enabled, string(cfg), c.Mode, string(c.Filter), c.RatePerMin) if err != nil { return 0, err } @@ -393,6 +402,11 @@ FROM notification_channels WHERE enabled ORDER BY id`) if err := rows.Scan(&c.ID, &c.Name, &c.Kind, &c.Config, &c.Mode, &c.Filter, &c.RatePerMin); err != nil { return nil, err } + cfg, err := openSecret(colNotifConfig, string(c.Config)) + if err != nil { + return nil, err + } + c.Config = json.RawMessage(cfg) out = append(out, &c) } return out, rows.Err() diff --git a/db/notification_delivery.go b/db/notification_delivery.go index 1bccab8..1d17852 100644 --- a/db/notification_delivery.go +++ b/db/notification_delivery.go @@ -95,6 +95,11 @@ func scanNotificationDelivery(sc interface{ Scan(...any) error }) (*Notification &channel.ID, &channel.Name, &channel.Kind, &chEnabled, &channel.Config, &channel.Mode, &channel.Filter, &channel.RatePerMin); err != nil { return nil, err } + cfg, err := openSecret(colNotifConfig, string(channel.Config)) + if err != nil { + return nil, err + } + channel.Config = json.RawMessage(cfg) dl.LastError = lastErr.String if batchID.Valid { dl.BatchID = &batchID.Int64 diff --git a/db/schema.sql b/db/schema.sql index 900a362..abbfdf8 100644 --- a/db/schema.sql +++ b/db/schema.sql @@ -1439,7 +1439,7 @@ CREATE TABLE IF NOT EXISTS notification_channels ( -- dingtalk DingTalk / feishu Feishu (Lark) / wecom WeCom (WeChat Work) / webhook generic / telegram / email kind TEXT NOT NULL, enabled BOOLEAN NOT NULL DEFAULT true, - -- Credentials (stored in plaintext, masked when echoed to the UI; see maskChannelSecrets on the server side). + -- Credentials (encrypted at rest by db/secrets.go, masked when echoed to the UI; see notify.MaskConfig). -- The six channel kinds differ wildly in fields, so one JSONB column + strict per-kind validation on the Go -- side avoids a pile of NULL columns per channel: -- dingtalk {webhook,secret} diff --git a/db/secrets.go b/db/secrets.go new file mode 100644 index 0000000..84af5f5 --- /dev/null +++ b/db/secrets.go @@ -0,0 +1,328 @@ +package db + +import ( + "bytes" + "context" + "crypto/aes" + "crypto/cipher" + "crypto/rand" + "database/sql" + "encoding/base64" + "encoding/hex" + "encoding/json" + "errors" + "fmt" + "log" + "os" + "path/filepath" + "strings" + "sync/atomic" +) + +// Secrets at rest: AES-256-GCM, a random nonce per value, AAD "
.". +// 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. + +const ( + secretPrefix = "enc:v1:" + sealedJSONKey = "__artex_enc" + secretKeyEnv = "ARTEX_SECRET_KEY" + secretKeyFile = "secret.key" +) + +type secretColumn struct { + table, col string + json bool +} + +func (c secretColumn) aad() string { return c.table + "." + c.col } + +// valueExpr is the SQL text that holds the ciphertext, if any. +func (c secretColumn) valueExpr() string { + if c.json { + return c.col + "->>'" + sealedJSONKey + "'" + } + return c.col +} + +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} +) + +type secretBox struct{ aead cipher.AEAD } + +var activeSecretBox atomic.Pointer[secretBox] + +func newSecretBox(key []byte) (*secretBox, error) { // key: 32 bytes, see parseSecretKey + block, err := aes.NewCipher(key) + if err != nil { + return nil, err + } + aead, err := cipher.NewGCM(block) + return &secretBox{aead}, err +} + +func isEncrypted(v string) bool { return strings.HasPrefix(v, secretPrefix) } + +func (b *secretBox) encrypt(aad, plain string) (string, error) { + if plain == "" { + return "", nil + } + nonce := make([]byte, b.aead.NonceSize()) + if _, err := rand.Read(nonce); err != nil { + return "", err + } + return secretPrefix + base64.StdEncoding.EncodeToString(b.aead.Seal(nonce, nonce, []byte(plain), []byte(aad))), nil +} + +func (b *secretBox) decrypt(aad, v string) (string, error) { + raw, err := base64.StdEncoding.DecodeString(strings.TrimPrefix(v, secretPrefix)) + n := b.aead.NonceSize() + if err != nil || len(raw) < n { + return "", fmt.Errorf("decrypt %s: malformed ciphertext", aad) + } + plain, err := b.aead.Open(nil, raw[:n], raw[n:], []byte(aad)) + if err != nil { + return "", fmt.Errorf("decrypt %s: authentication failed (wrong key or altered value)", aad) + } + return string(plain), nil +} + +// sealedJSON returns the ciphertext of a {"__artex_enc":"enc:v1:..."} envelope. +func sealedJSON(raw string) (string, bool) { + if !strings.Contains(raw, `"`+sealedJSONKey+`"`) { + return "", false + } + var m map[string]json.RawMessage + var s string + if json.Unmarshal([]byte(raw), &m) != nil || len(m) != 1 || json.Unmarshal(m[sealedJSONKey], &s) != nil || !isEncrypted(s) { + return "", false + } + return s, true +} + +func isSealed(c secretColumn, v string) bool { + if c.json { + _, ok := sealedJSON(v) + return ok + } + return isEncrypted(v) +} + +// seal encrypts one column value. JSON is canonicalized first; an empty object +// or null stays as is, and a top-level key named like the envelope is refused. +func (b *secretBox) seal(c secretColumn, v string) (string, error) { + if !c.json { + return b.encrypt(c.aad(), v) + } + dec := json.NewDecoder(bytes.NewReader([]byte(v))) + dec.UseNumber() + var parsed any + if err := dec.Decode(&parsed); err != nil { + return "", fmt.Errorf("%s: invalid JSON", c.aad()) + } + m, isObject := parsed.(map[string]any) + if _, reserved := m[sealedJSONKey]; reserved { + return "", fmt.Errorf("%s: key %q is reserved", c.aad(), sealedJSONKey) + } + if parsed == nil || (isObject && len(m) == 0) { + return v, nil // {} or null: nothing to hide + } + canon, err := json.Marshal(parsed) + if err != nil { + return "", err + } + enc, err := b.encrypt(c.aad(), string(canon)) + if err != nil { + return "", err + } + out, err := json.Marshal(map[string]string{sealedJSONKey: enc}) + return string(out), err +} + +func (b *secretBox) open(c secretColumn, v string) (string, error) { + if c.json { + v, _ = sealedJSON(v) + } + return b.decrypt(c.aad(), v) +} + +var errNoSecretKey = errors.New("secrets at rest: key not initialized") + +// sealSecret encrypts a value before it is written to column c. +func sealSecret(c secretColumn, v string) (string, error) { + if b := activeSecretBox.Load(); b != nil { + return b.seal(c, v) + } + return "", errNoSecretKey +} + +// openSecret decrypts a value read from column c. A value that is not sealed is +// a legacy plaintext row (the startup backfill encrypts those) and passes through. +func openSecret(c secretColumn, v string) (string, error) { + if !isSealed(c, v) { + return v, nil + } + if b := activeSecretBox.Load(); b != nil { + return b.open(c, v) + } + return "", errNoSecretKey +} + +// parseSecretKey accepts 32 bytes encoded as base64 or hex. +func parseSecretKey(s string) ([]byte, error) { + s = strings.TrimSpace(s) + if k, err := base64.StdEncoding.DecodeString(s); err == nil && len(k) == 32 { + return k, nil + } + if k, err := hex.DecodeString(s); err == nil && len(k) == 32 { + return k, nil + } + return nil, errors.New("must be 32 bytes, base64 or hex encoded") +} + +// InitSecrets loads the key (ARTEX_SECRET_KEY, else keyDir/secret.key, else a new +// one, refused when encrypted data exists) and encrypts legacy plaintext, under +// the schema advisory lock so concurrent instances backfill once. +func (d *DB) InitSecrets(keyDir string) error { + ctx := context.Background() + return withSchemaMigrationLock(ctx, d.DB, func(conn *sql.Conn) error { + key, err := loadSecretKey(ctx, conn, keyDir) + if err != nil { + return err + } + box, err := newSecretBox(key) + if err != nil { + return err + } + // 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) + if err == nil { + _, err = box.decrypt(c.aad(), v) + } + if err != nil && err != sql.ErrNoRows { + return fmt.Errorf("secret key check: %w", err) + } + } + n, err := backfillSecrets(ctx, conn, box) + if err != nil { + return err + } + log.Printf("[secrets] at-rest encryption ready; backfill encrypted %d plaintext value(s)", n) + activeSecretBox.Store(box) + return nil + }) +} + +func loadSecretKey(ctx context.Context, conn *sql.Conn, keyDir string) ([]byte, error) { + if v := strings.TrimSpace(os.Getenv(secretKeyEnv)); v != "" { + k, err := parseSecretKey(v) + if err != nil { + return nil, fmt.Errorf("%s %w", secretKeyEnv, err) + } + return k, nil + } + path := filepath.Join(keyDir, secretKeyFile) + if data, err := os.ReadFile(path); err == nil { + k, err := parseSecretKey(string(data)) + if err != nil { + return nil, fmt.Errorf("secret key %s %w", path, err) + } + return k, nil + } else if !os.IsNotExist(err) { + return nil, fmt.Errorf("read secret key: %w", err) + } + var parts []string + for _, c := range secretColumns { + parts = append(parts, `(SELECT count(*) FROM `+c.table+` WHERE `+c.valueExpr()+` LIKE 'enc:v1:%')`) + } + var n int64 + if err := conn.QueryRowContext(ctx, `SELECT `+strings.Join(parts, "+")).Scan(&n); err != nil { + return nil, err + } + if n > 0 { + return nil, fmt.Errorf("the database holds %d encrypted secret(s) but no key is available: set %s or restore %s (refusing to generate a new key over encrypted data)", n, secretKeyEnv, path) + } + key := make([]byte, 32) + if _, err := rand.Read(key); err != nil { + return nil, err + } + if err := os.MkdirAll(keyDir, 0o700); err != nil { + return nil, fmt.Errorf("create secret key dir: %w", err) + } + f, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0o600) // never overwrite a key + if err != nil { + return nil, fmt.Errorf("write secret key: %w", err) + } + if _, err = f.WriteString(base64.StdEncoding.EncodeToString(key) + "\n"); err == nil { + err = f.Close() + } + if err != nil { + f.Close() + _ = os.Remove(path) + return nil, fmt.Errorf("write secret key: %w", err) + } + log.Printf("[secrets] new secret key written to %s", path) + return key, nil +} + +// backfillSecrets encrypts every plaintext value once; sealed values are +// skipped, so it is idempotent. Each UPDATE matches the value it read, so a +// concurrent write is never overwritten. +func backfillSecrets(ctx context.Context, conn *sql.Conn, box *secretBox) (int, error) { + total := 0 + for _, c := range secretColumns { + var ids []int64 + var olds, news []string + rows, err := conn.QueryContext(ctx, `SELECT id, `+c.col+`::text FROM `+c.table+` WHERE `+c.col+` IS NOT NULL`) + if err != nil { + return total, err + } + for rows.Next() { + var id int64 + var old string + if err = rows.Scan(&id, &old); err != nil { + break + } + if isSealed(c, old) { + continue + } + nv, serr := box.seal(c, old) + if serr != nil { + err = fmt.Errorf("backfill %s id=%d: %w", c.aad(), id, serr) + break + } + if isSealed(c, nv) { // empty values stay as they are + ids, olds, news = append(ids, id), append(olds, old), append(news, nv) + } + } + rows.Close() + if err == nil { + err = rows.Err() + } + if err != nil { + return total, err + } + cast := "" + if c.json { + 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]) + if err != nil { + return total, fmt.Errorf("backfill %s id=%d: %w", c.aad(), id, err) + } + if n, _ := res.RowsAffected(); n > 0 { + total++ + } + } + } + return total, nil +} diff --git a/db/secrets_test.go b/db/secrets_test.go new file mode 100644 index 0000000..e1da516 --- /dev/null +++ b/db/secrets_test.go @@ -0,0 +1,197 @@ +package db + +import ( + "context" + "encoding/base64" + "encoding/hex" + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" +) + +var testSecretKey = []byte("0123456789abcdef0123456789abcdef") + +func TestSecretBoxCrypto(t *testing.T) { + b, _ := newSecretBox(testSecretKey) + c1, err := b.encrypt("t.c", "hunter2") + if err != nil { + t.Fatal(err) + } + c2, _ := b.encrypt("t.c", "hunter2") + if !isEncrypted(c1) || isEncrypted("hunter2") || c1 == c2 || strings.Contains(c1, "hunter2") { + t.Fatalf("want distinct enc:v1: ciphertexts, got %q %q", c1, c2) + } + if p, err := b.decrypt("t.c", c1); err != nil || p != "hunter2" { + t.Fatalf("round trip: %q %v", p, err) + } + if _, err := b.decrypt("t.other", c1); err == nil { + t.Fatal("wrong AAD must fail") + } + raw, _ := base64.StdEncoding.DecodeString(strings.TrimPrefix(c1, secretPrefix)) + raw[len(raw)-1] ^= 1 + if _, err := b.decrypt("t.c", secretPrefix+base64.StdEncoding.EncodeToString(raw)); err == nil { + t.Fatal("tampered ciphertext must fail") + } + if v, _ := b.encrypt("t.c", ""); v != "" { + t.Fatal("empty must stay empty") + } + sealed, err := b.seal(colMCPEnv, `{"b":1, "a":"tok"}`) + if err != nil || !isSealed(colMCPEnv, sealed) || strings.Contains(sealed, "tok") { + t.Fatalf("not sealed: %s %v", sealed, err) + } + if p, err := b.open(colMCPEnv, sealed); err != nil || p != `{"a":"tok","b":1}` { + t.Fatalf("canonical JSON round trip: %q %v", p, err) + } + if _, err := b.seal(colMCPEnv, `{"__artex_enc":"x"}`); err == nil { + t.Fatal("reserved key must be refused") + } + if out, _ := b.seal(colMCPEnv, `{}`); out != `{}` { + t.Fatalf("empty object must stay empty, got %s", out) + } +} + +type secretRows struct{ llm, mcp, cred, ch, task int64 } + +var plainSecrets = []string{"sk-plain-llm", "mcp-plain-tok", "plain-pass", "hook-plain-secret"} + +// secretFixture writes one secret to each location, through the db API or, for +// a legacy database, as raw plaintext SQL. +func secretFixture(t *testing.T, d *DB, legacy bool) (r secretRows) { + t.Helper() + task, err := d.CreateTask("secrets at rest", "goal", nil, 0, 0) + if err != nil { + t.Fatal(err) + } + r.task = task.ID + t.Cleanup(func() { + _ = d.DeleteTask(r.task) + 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) + }) + env, cfg := `{"Authorization":"mcp-plain-tok"}`, `{"url":"https://h.example","headers":{"X":"hook-plain-secret"}}` + if legacy { + ins := func(id *int64, q string, args ...any) { + if err := d.QueryRow(q, args...).Scan(id); err != nil { + 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.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"}) + if err == nil { + r.mcp, err = d.SaveMCP(&MCPServer{Name: "t-sec", Transport: "http", Env: json.RawMessage(env)}) + } + if err == nil { + var c TaskCredential + c, err = d.UpsertTaskCredential(r.task, TaskCredentialInput{Domain: "x.example", Username: "u", Password: "plain-pass"}) + r.cred = c.ID + } + if err == nil { + r.ch, err = d.SaveNotificationChannel(context.Background(), &NotificationChannel{Name: "t-sec", Kind: "webhook", Config: json.RawMessage(cfg)}) + } + if err != nil { + t.Fatal(err) + } + return r +} + +// rawSecrets reads the four columns as stored in PostgreSQL. +func rawSecrets(t *testing.T, d *DB, r secretRows) string { + t.Helper() + var a, b, c, e 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 { + t.Fatal(err) + } + if !isEncrypted(a) || !isSealed(colMCPEnv, b) || !isEncrypted(c) || !isSealed(colNotifConfig, e) { + t.Fatal("a stored secret is not encrypted") + } + all := strings.Join([]string{a, b, c, e}, "|") + for _, p := range plainSecrets { + if strings.Contains(all, p) { + t.Fatalf("plaintext %q found at rest", p) + } + } + return all +} + +// assertPlainReads checks that every db read path returns the decrypted secret. +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 + } + ms, _ := d.ListMCP() + for _, m := range ms { + if m.ID == r.mcp { + got += string(m.Env) + } + } + if creds, err := d.TaskCredentials(r.task); err == nil && len(creds) == 1 { + got += creds[0].Password + } + if ch, err := d.NotificationChannelByID(context.Background(), r.ch); err == nil { + got += string(ch.Config) + } + for _, p := range plainSecrets { + if !strings.Contains(got, p) { + t.Fatalf("%q not read back in clear", p) + } + } +} + +func TestSecretsAtRestWriteRead(t *testing.T) { + d, err := Open(testDSN(t)) + if err != nil { + t.Skipf("postgres unavailable (%v) — skipping", err) + } + t.Cleanup(func() { d.Close() }) // after the fixture cleanup + r := secretFixture(t, d, false) + rawSecrets(t, d, r) + assertPlainReads(t, d, r) + if _, err := d.SaveMCP(&MCPServer{Name: "t-sec-reserved", Transport: "http", Env: json.RawMessage(`{"__artex_enc":"x"}`)}); err == nil { + t.Fatal("reserved key must be refused on write") + } +} + +func TestSecretsBackfillLegacyAndKeyChecks(t *testing.T) { + d, err := Open(testDSN(t)) + if err != nil { + t.Skipf("postgres unavailable (%v) — skipping", err) + } + t.Cleanup(func() { d.Close() }) // after the fixture cleanup + r := secretFixture(t, d, true) + t.Setenv(secretKeyEnv, base64.StdEncoding.EncodeToString(testSecretKey)) + if err := d.InitSecrets(t.TempDir()); err != nil { + t.Fatal(err) + } + first := rawSecrets(t, d, r) + assertPlainReads(t, d, r) + if err := d.InitSecrets(t.TempDir()); err != nil || rawSecrets(t, d, r) != first { + t.Fatalf("second backfill must leave ciphertexts untouched (%v)", err) + } + + // Encrypted data, no env key, no key file: refuse, and write no new key. + t.Setenv(secretKeyEnv, "") + dir := t.TempDir() + if err := d.InitSecrets(dir); err == nil || !strings.Contains(err.Error(), "no key is available") { + t.Fatalf("want refusal without a key, got %v", err) + } + if _, err := os.Stat(filepath.Join(dir, secretKeyFile)); !os.IsNotExist(err) { + t.Fatal("a new key was generated over encrypted data") + } + t.Setenv(secretKeyEnv, hex.EncodeToString([]byte("ffffffffffffffffffffffffffffffff"))) + if err := d.InitSecrets(dir); err == nil || !strings.Contains(err.Error(), "secret key check") { + t.Fatalf("want wrong-key refusal, got %v", err) + } + assertPlainReads(t, d, r) // the active key is unchanged +} diff --git a/db/task_credentials.go b/db/task_credentials.go index 2a49d10..306f5bd 100644 --- a/db/task_credentials.go +++ b/db/task_credentials.go @@ -10,9 +10,8 @@ import ( // task creation (source=user, e.g. a program-provided alias account) or registered // by the agent itself and recorded so it is not forgotten (source=agent). // -// Passwords are stored in the local Postgres, plaintext — the same trust domain as -// captured traffic evidence, which already contains credentials by nature. The UI -// masks them by default; anything injected into agent context transits to the LLM +// Passwords are encrypted at rest (secrets.go) and masked by the HTTP API; the +// agent tools read them decrypted. Anything injected into agent context transits to the LLM // provider, so only dedicated test accounts belong here. type TaskCredential struct { ID int64 `json:"id"` @@ -64,6 +63,10 @@ func (d *DB) UpsertTaskCredential(taskID int64, in TaskCredentialInput) (TaskCre if err != nil { return TaskCredential{}, err } + pass, err := sealSecret(colCredPass, in.Password) + if err != nil { + return TaskCredential{}, err + } var c TaskCredential err = d.QueryRow(` INSERT INTO task_credentials (task_id, domain, username, password, source, note) @@ -74,8 +77,9 @@ func (d *DB) UpsertTaskCredential(taskID int64, in TaskCredentialInput) (TaskCre note = CASE WHEN EXCLUDED.note = '' THEN task_credentials.note ELSE EXCLUDED.note END, updated_at = now() RETURNING id, task_id, domain, username, password, source, note, created_at, updated_at`, - taskID, in.Domain, in.Username, in.Password, in.Source, in.Note). + taskID, in.Domain, in.Username, pass, in.Source, in.Note). Scan(&c.ID, &c.TaskID, &c.Domain, &c.Username, &c.Password, &c.Source, &c.Note, &c.CreatedAt, &c.UpdatedAt) + c.Password = in.Password return c, err } @@ -95,6 +99,9 @@ func (d *DB) TaskCredentials(taskID int64) ([]TaskCredential, error) { if err := rows.Scan(&c.ID, &c.TaskID, &c.Domain, &c.Username, &c.Password, &c.Source, &c.Note, &c.CreatedAt, &c.UpdatedAt); err != nil { return nil, err } + if c.Password, err = openSecret(colCredPass, c.Password); err != nil { + return nil, err + } out = append(out, c) } return out, rows.Err() diff --git a/db/testmain_test.go b/db/testmain_test.go index 588ce2a..0efff92 100644 --- a/db/testmain_test.go +++ b/db/testmain_test.go @@ -18,6 +18,8 @@ func TestMain(m *testing.M) { // contending on shared tables and the advisory lock below stops // serializing go test ./... across packages. SuffixTestDatabase("db") + box, _ := newSecretBox(testSecretKey) + activeSecretBox.Store(box) dsn, _, err := DSN() if err != nil { // No DB configured — tests that need PG will skip themselves. diff --git a/docker-compose.yml b/docker-compose.yml index 58f7803..d87ee11 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -35,6 +35,8 @@ services: ARTEX_LLM_BASE_URL: ${ARTEX_LLM_BASE_URL:-} ARTEX_LLM_PROXY: ${ARTEX_LLM_PROXY:-} ARTEX_JWT_KEY_DIR: /app/keys + # Optional: key for secrets at rest; empty = keys/secret.key (generated at first start). + ARTEX_SECRET_KEY: ${ARTEX_SECRET_KEY:-} ARTEX_TRUSTED_PROXIES: ${ARTEX_TRUSTED_PROXIES:-} # Untrusted-output boundary: on by default (empty = on); ARTEX_UNTRUSTED_BOUNDARY=0 in .env # disables it. Cost: about 154 bytes per tool result plus one system note. diff --git a/server/env_scrub.go b/server/env_scrub.go index e3c695e..44e57a1 100644 --- a/server/env_scrub.go +++ b/server/env_scrub.go @@ -3,8 +3,8 @@ package server import "os" // secretEnv lists the credentials read from the environment only at startup: -// ARTEX_PG_DSN by NewManager, the API keys by agent.FromEnv inside New. -var secretEnv = []string{"ARTEX_PG_DSN", "ANTHROPIC_API_KEY", "OPENAI_API_KEY"} +// ARTEX_PG_DSN and ARTEX_SECRET_KEY by NewManager, the API keys by agent.FromEnv inside New. +var secretEnv = []string{"ARTEX_PG_DSN", "ARTEX_SECRET_KEY", "ANTHROPIC_API_KEY", "OPENAI_API_KEY"} // ScrubSecretEnv removes secretEnv from the process environment once startup has // read it. Child processes (agent shells, custom tools, MCP stdio servers) copy diff --git a/server/env_scrub_test.go b/server/env_scrub_test.go index 0fca0fc..d840869 100644 --- a/server/env_scrub_test.go +++ b/server/env_scrub_test.go @@ -46,6 +46,7 @@ func TestNewAppliesSecretScrubOption(t *testing.T) { if dsn := os.Getenv("ARTEX_PG_DSN"); dsn != "" { t.Setenv("ARTEX_PG_DSN", dsn) } + t.Setenv("ARTEX_SECRET_KEY", os.Getenv("ARTEX_SECRET_KEY")) t.Setenv("ANTHROPIC_API_KEY", "scrub-probe") t.Setenv("OPENAI_API_KEY", "scrub-probe") diff --git a/server/manager.go b/server/manager.go index 63b9358..f60340c 100644 --- a/server/manager.go +++ b/server/manager.go @@ -17,6 +17,7 @@ import ( "time" "github.com/Autumn-27/artex/agent" + "github.com/Autumn-27/artex/config" pgdb "github.com/Autumn-27/artex/db" "github.com/Autumn-27/artex/enrich" "github.com/Autumn-27/artex/guard" @@ -391,6 +392,11 @@ func NewManager(dir, proxyAddr string) (*Manager, error) { if err != nil { return nil, err } + // Before anything reads a stored secret: load the key, encrypt legacy plaintext. + if err := pg.InitSecrets(config.KeyDir()); err != nil { + pg.Close() + return nil, fmt.Errorf("secrets at rest: %w", err) + } if err := pg.RecoverFindingRetests(); err != nil { pg.Close() return nil, fmt.Errorf("recover finding retests: %w", err) diff --git a/server/secrets_api_test.go b/server/secrets_api_test.go new file mode 100644 index 0000000..77ff1a1 --- /dev/null +++ b/server/secrets_api_test.go @@ -0,0 +1,83 @@ +package server + +import ( + "context" + "encoding/json" + "strconv" + "strings" + "testing" + + "github.com/Autumn-27/artex/db" +) + +// No read endpoint returns a stored secret in clear, and a masked placeholder +// sent back on write never replaces the stored secret. +func TestSecretsNeverReturnedInClear(t *testing.T) { + s, _, request := trafficEvidenceServer(t) + pg := s.m.PG() + secrets := []string{"sk-llm-plain-0001", "mcp-env-plain-0002", "cred-plain-0003", "SECding-plain-0004", "tok-plain-0005"} + task, err := s.m.CreateTask("secrets api", "goal", nil, 0, 0) + if err != nil { + t.Fatal(err) + } + taskID, _ := strconv.ParseInt(task.ID, 10, 64) + llmID, err := pg.SaveProfile(&db.LLMProfile{Name: "t-secapi", Format: "openai", Model: "m", APIKey: secrets[0]}) + mcpID, err2 := pg.SaveMCP(&db.MCPServer{Name: "t-secapi", Transport: "http", Env: json.RawMessage(`{"X-API-Key":"` + secrets[1] + `","EMPTY":""}`)}) + _, err3 := pg.UpsertTaskCredential(taskID, db.TaskCredentialInput{Domain: "x.example", Username: "u", Password: secrets[2]}) + chID, err4 := pg.SaveNotificationChannel(context.Background(), &db.NotificationChannel{Name: "t-secapi", Kind: "dingtalk", + Config: json.RawMessage(`{"webhook":"https://oapi.example/send?access_token=` + secrets[4] + `","secret":"` + secrets[3] + `"}`)}) + t.Cleanup(func() { + pg.Exec(`DELETE FROM llm_profiles WHERE id=$1`, llmID) + pg.Exec(`DELETE FROM mcp_servers WHERE id=$1`, mcpID) + pg.Exec(`DELETE FROM notification_channels WHERE id=$1`, chID) + pg.DeleteTask(taskID) + }) + for _, e := range []error{err, err2, err3, err4} { + if e != nil { + t.Fatal(e) + } + } + + for _, path := range []string{"/api/llm/profiles", "/api/llm", "/api/mcp", "/api/notify/channels", "/api/tasks/" + task.ID + "/credentials"} { + resp := request("GET", path, "") + for _, sec := range secrets { + if resp.Code != 200 || strings.Contains(resp.Body.String(), sec) { + t.Fatalf("GET %s: HTTP %d or a secret returned in clear", path, resp.Code) + } + } + } + + // MCP: echo the masked list entry back with a name change; the key survives. + var list struct{ Servers []db.MCPServer } + _ = json.Unmarshal(request("GET", "/api/mcp", "").Body.Bytes(), &list) + for _, m := range list.Servers { + if m.ID != mcpID { + continue + } + if string(m.Env) != `{"EMPTY":"","X-API-Key":"__masked__"}` { + t.Fatalf("mcp env not masked as expected: %s", m.Env) + } + m.Name = "t-secapi-renamed" + body, _ := json.Marshal(m) + if resp := request("POST", "/api/mcp", string(body)); resp.Code != 200 { + t.Fatalf("mcp save: HTTP %d %s", resp.Code, resp.Body.String()) + } + } + all, _ := pg.ListMCP() + for _, m := range all { + if m.ID == mcpID && !strings.Contains(string(m.Env), secrets[1]) { + t.Fatal("masked placeholder replaced the stored MCP secret") + } + } + if resp := request("POST", "/api/mcp", `{"name":"t-secapi-new","transport":"http","env":{"K":"__masked__"}}`); resp.Code != 400 { + t.Fatalf("a placeholder with nothing stored must be refused, got HTTP %d", resp.Code) + } + + // Task credential: a masked password on write is refused, the stored one kept. + if resp := request("POST", "/api/tasks/"+task.ID+"/credentials", `{"domain":"x.example","username":"u","password":"__masked__"}`); resp.Code != 400 { + t.Fatalf("masked credential write must be refused, got HTTP %d", resp.Code) + } + if creds, _ := pg.TaskCredentials(taskID); len(creds) != 1 || creds[0].Password != secrets[2] { + t.Fatal("stored credential changed") + } +} diff --git a/server/server_mgmt.go b/server/server_mgmt.go index 3a378df..e4d48fe 100644 --- a/server/server_mgmt.go +++ b/server/server_mgmt.go @@ -25,6 +25,7 @@ import ( "github.com/Autumn-27/artex/agent" "github.com/Autumn-27/artex/db" + "github.com/Autumn-27/artex/notify" "github.com/Autumn-27/norma/llm" "github.com/Autumn-27/norma/skill" ) @@ -857,9 +858,57 @@ func (s *Server) pgListMCP(w http.ResponseWriter, r *http.Request) { if ms == nil { ms = []*db.MCPServer{} } + for _, m := range ms { + m.Env = maskMCPEnv(m.Env) + } writeJSON(w, 200, map[string]any{"servers": ms}) } +// 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 { + var env map[string]any + if json.Unmarshal(raw, &env) != nil || env == nil { + return json.RawMessage(`{}`) + } + for k, v := range env { + if v != "" { + env[k] = notify.MaskedPrefix + } + } + out, _ := json.Marshal(env) + return out +} + +// unmaskMCPEnv puts the stored value back for every env key sent as the mask, +// so a client echoing the list response never overwrites the stored secret. +func unmaskMCPEnv(pg *db.DB, m *db.MCPServer) error { + var env, stored map[string]any + if json.Unmarshal(m.Env, &env) != nil { + return nil // not an object: SaveMCP stores or rejects it as before + } + all, err := pg.ListMCP() + if err != nil { + return err + } + for _, cur := range all { + if m.ID != 0 && cur.ID == m.ID { + _ = json.Unmarshal(cur.Env, &stored) + } + } + for k, v := range env { + if s, ok := v.(string); ok && notify.IsMasked(s) { + old, ok := stored[k] + if !ok { + return fmt.Errorf("env %q: masked value but nothing stored to keep", k) + } + env[k] = old + } + } + m.Env, err = json.Marshal(env) + return err +} + func (s *Server) pgSaveMCP(w http.ResponseWriter, r *http.Request) { pg := s.pg(w) if pg == nil { @@ -871,6 +920,10 @@ func (s *Server) pgSaveMCP(w http.ResponseWriter, r *http.Request) { return } isNew := m.ID == 0 + if err := unmaskMCPEnv(pg, &m); err != nil { + writeErr(w, 400, err.Error()) + return + } id, err := pg.SaveMCP(&m) if err != nil { writeErr(w, 500, err.Error()) diff --git a/server/task_credentials.go b/server/task_credentials.go index 26c56b9..ca1210d 100644 --- a/server/task_credentials.go +++ b/server/task_credentials.go @@ -6,18 +6,23 @@ import ( "strconv" "github.com/Autumn-27/artex/db" + "github.com/Autumn-27/artex/notify" ) // Task credential HTTP handlers (#3). The agent-side write path is the // save_task_credential tool; these routes serve the UI (task detail tab) and the // creation dialog. -// taskCredentialDTO masks nothing — the admin is the credential owner — but the -// frontend renders passwords masked with an explicit reveal. +// taskCredentialDTO masks the password server-side; an empty password stays +// empty (the "agent generates" mode). Agent tools read the stored value directly. func taskCredentialDTO(c db.TaskCredential) map[string]any { + pass := c.Password + if pass != "" { + pass = notify.MaskedPrefix + } return map[string]any{ "id": c.ID, "task_id": c.TaskID, "domain": c.Domain, "username": c.Username, - "password": c.Password, "source": c.Source, "note": c.Note, + "password": pass, "source": c.Source, "note": c.Note, "created_at": c.CreatedAt, "updated_at": c.UpdatedAt, } } @@ -63,6 +68,10 @@ func (s *Server) addTaskCredential(w http.ResponseWriter, r *http.Request) { writeErr(w, 400, "bad json: "+err.Error()) return } + if notify.IsMasked(req.Password) { + writeErr(w, 400, "password is a masked placeholder; enter the real password") + return + } // source=user: manual adds from the UI (e.g. relaying an OTP-verified account). // An empty password stays allowed here — the "agent generates" mode. c, err := s.m.PG().UpsertTaskCredential(id, db.TaskCredentialInput{ diff --git a/server/task_credentials_test.go b/server/task_credentials_test.go index 215b3af..5376ac3 100644 --- a/server/task_credentials_test.go +++ b/server/task_credentials_test.go @@ -63,7 +63,7 @@ func TestTaskCredentialsFlow(t *testing.T) { if len(list) != 2 { t.Fatalf("want 2 credentials, got %d: %s", len(list), resp.Body.String()) } - if list[0].Domain != "target.example.com" || list[0].Username != "alias@test" || list[0].Password != "s3cret!" || list[0].Note != "program alias account" { + if list[0].Domain != "target.example.com" || list[0].Username != "alias@test" || list[0].Password != "__masked__" || list[0].Note != "program alias account" { t.Fatalf("declared credential mismatch: %+v", list[0]) } if list[1].Password != "" { diff --git a/server/testmain_test.go b/server/testmain_test.go index bec4cf3..817f2f3 100644 --- a/server/testmain_test.go +++ b/server/testmain_test.go @@ -17,6 +17,8 @@ func TestMain(m *testing.M) { // contending on shared tables and the advisory lock below stops // serializing go test ./... across packages. db.SuffixTestDatabase("server") + // Fixed test key via env: NewManager's InitSecrets then writes no key file. + os.Setenv("ARTEX_SECRET_KEY", "MDEyMzQ1Njc4OWFiY2RlZjAxMjM0NTY3ODlhYmNkZWY=") dsn, _, err := db.DSN() if err != nil { os.Exit(m.Run()) From 4f785a8b4f09d83912a71d7fea70112790735e8f Mon Sep 17 00:00:00 2001 From: BaaRai <130491490+BaaRai@users.noreply.github.com> Date: Fri, 13 Nov 2026 15:02:47 +0100 Subject: [PATCH 2/2] Guard masked MCP env against a destination change A masked env value keeps the stored secret only while transport, command, args (canonical JSON) and url are unchanged, as notify.PrepareConfigUpdate does for channels. A changed destination with a masked value is refused (400). The credentials tab drops reveal/copy and shows a stored password as "stored, hidden". --- server/secrets_api_test.go | 63 +++++++++++++++++++ server/server_mgmt.go | 29 ++++++++- .../tasks/detail/_tabs/credentials-tab.tsx | 32 +--------- 3 files changed, 92 insertions(+), 32 deletions(-) diff --git a/server/secrets_api_test.go b/server/secrets_api_test.go index 77ff1a1..1266654 100644 --- a/server/secrets_api_test.go +++ b/server/secrets_api_test.go @@ -81,3 +81,66 @@ func TestSecretsNeverReturnedInClear(t *testing.T) { t.Fatal("stored credential changed") } } + +// A masked MCP env value is only kept while the destination (transport, +// command, args, url) is unchanged; otherwise the secrets must be re-entered. +func TestMCPMaskedEnvDestinationGuard(t *testing.T) { + s, _, request := trafficEvidenceServer(t) + pg := s.m.PG() + const secret = "mcp-guard-secret-0006" + httpID, err := pg.SaveMCP(&db.MCPServer{Name: "t-guard-http", Transport: "http", URL: "http://127.0.0.1:9/a", + Env: json.RawMessage(`{"K":"` + secret + `"}`)}) + if err != nil { + t.Fatal(err) + } + stdioID, err := pg.SaveMCP(&db.MCPServer{Name: "t-guard-stdio", Transport: "stdio", Command: "npx", + Args: json.RawMessage(`["pkg"]`), Env: json.RawMessage(`{"K":"` + secret + `"}`)}) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { pg.DeleteMCP(httpID); pg.DeleteMCP(stdioID) }) + stored := func(id int64) db.MCPServer { + all, _ := pg.ListMCP() + for _, m := range all { + if m.ID == id { + return *m + } + } + t.Fatalf("mcp %d missing", id) + return db.MCPServer{} + } + save := func(id int64, edit func(*db.MCPServer), want int) { + t.Helper() + m := stored(id) + m.Env = maskMCPEnv(m.Env) + edit(&m) + body, _ := json.Marshal(m) + if resp := request("POST", "/api/mcp", string(body)); resp.Code != want { + t.Fatalf("want HTTP %d, got %d %s", want, resp.Code, resp.Body.String()) + } + } + keeps := func(id int64, url string) { + t.Helper() + if m := stored(id); !strings.Contains(string(m.Env), secret) || m.URL != url { + t.Fatalf("stored row changed: url=%q", m.URL) + } + } + + save(httpID, func(m *db.MCPServer) { m.Name = "t-guard-http2" }, 200) + keeps(httpID, "http://127.0.0.1:9/a") + save(httpID, func(m *db.MCPServer) { m.URL = "http://127.0.0.1:9/evil" }, 400) + keeps(httpID, "http://127.0.0.1:9/a") + save(httpID, func(m *db.MCPServer) { m.Transport = "sse" }, 400) + keeps(httpID, "http://127.0.0.1:9/a") + save(stdioID, func(m *db.MCPServer) { m.Command = "sh" }, 400) + save(stdioID, func(m *db.MCPServer) { m.Args = json.RawMessage(`["other"]`) }, 400) + keeps(stdioID, "") + save(stdioID, func(m *db.MCPServer) { m.Args = json.RawMessage(`[ "pkg" ]`) }, 200) // same args, other spacing + keeps(stdioID, "") + save(httpID, func(m *db.MCPServer) { + m.URL, m.Env = "http://127.0.0.1:9/b", json.RawMessage(`{"K":"new-explicit-value"}`) + }, 200) + if m := stored(httpID); m.URL != "http://127.0.0.1:9/b" || !strings.Contains(string(m.Env), "new-explicit-value") { + t.Fatal("explicit env with a new url was not saved") + } +} diff --git a/server/server_mgmt.go b/server/server_mgmt.go index e4d48fe..c236627 100644 --- a/server/server_mgmt.go +++ b/server/server_mgmt.go @@ -882,8 +882,11 @@ func maskMCPEnv(raw json.RawMessage) json.RawMessage { // unmaskMCPEnv puts the stored value back for every env key sent as the mask, // so a client echoing the list response never overwrites the stored secret. +// As in notify.PrepareConfigUpdate, a stored secret is only kept for the +// destination it was configured for: when transport, command, args or url +// change, every env value must be re-entered (or cleared) explicitly. func unmaskMCPEnv(pg *db.DB, m *db.MCPServer) error { - var env, stored map[string]any + var env, storedEnv map[string]any if json.Unmarshal(m.Env, &env) != nil { return nil // not an object: SaveMCP stores or rejects it as before } @@ -891,17 +894,24 @@ func unmaskMCPEnv(pg *db.DB, m *db.MCPServer) error { if err != nil { return err } + var stored *db.MCPServer for _, cur := range all { if m.ID != 0 && cur.ID == m.ID { - _ = json.Unmarshal(cur.Env, &stored) + stored = cur + _ = json.Unmarshal(cur.Env, &storedEnv) } } + moved := stored != nil && (stored.Transport != m.Transport || stored.Command != m.Command || + stored.URL != m.URL || canonicalMCPArgs(stored.Args) != canonicalMCPArgs(m.Args)) for k, v := range env { if s, ok := v.(string); ok && notify.IsMasked(s) { - old, ok := stored[k] + old, ok := storedEnv[k] if !ok { return fmt.Errorf("env %q: masked value but nothing stored to keep", k) } + if moved { + return fmt.Errorf("the MCP destination (transport, command, args or url) changed: re-enter the env values (%q is masked) or clear them; stored secrets are not sent to a new destination", k) + } env[k] = old } } @@ -909,6 +919,19 @@ func unmaskMCPEnv(pg *db.DB, m *db.MCPServer) error { return err } +// canonicalMCPArgs normalizes args JSON for comparison; empty means [] as in SaveMCP. +func canonicalMCPArgs(raw json.RawMessage) string { + var v any + if len(raw) == 0 || string(raw) == "null" { + return "[]" + } + if json.Unmarshal(raw, &v) != nil { + return string(raw) + } + out, _ := json.Marshal(v) + return string(out) +} + func (s *Server) pgSaveMCP(w http.ResponseWriter, r *http.Request) { pg := s.pg(w) if pg == nil { diff --git a/web/src/app/(main)/function/tasks/detail/_tabs/credentials-tab.tsx b/web/src/app/(main)/function/tasks/detail/_tabs/credentials-tab.tsx index a8b8bf4..644f46e 100644 --- a/web/src/app/(main)/function/tasks/detail/_tabs/credentials-tab.tsx +++ b/web/src/app/(main)/function/tasks/detail/_tabs/credentials-tab.tsx @@ -2,7 +2,7 @@ import * as React from "react"; -import { CopyIcon, EyeIcon, EyeOffIcon, PlusIcon, TrashIcon } from "lucide-react"; +import { PlusIcon, TrashIcon } from "lucide-react"; import { Badge } from "@/components/ui/badge"; import { Button } from "@/components/ui/button"; @@ -15,11 +15,10 @@ import type { TaskCredential } from "@/lib/types"; // Credentials tab (#3): the task's test accounts — user-declared (program alias // accounts) and agent-recorded (accounts the workers registered themselves). -// Passwords render masked; reveal and copy are explicit. +// Passwords are masked server-side and never sent to the browser. export function CredentialsTab({ taskId }: { taskId: string }) { const [items, setItems] = React.useState(null); const [error, setError] = React.useState(""); - const [revealed, setRevealed] = React.useState>(new Set()); const [domain, setDomain] = React.useState(""); const [username, setUsername] = React.useState(""); const [password, setPassword] = React.useState(""); @@ -74,15 +73,6 @@ export function CredentialsTab({ taskId }: { taskId: string }) { } } - function toggle(id: number) { - setRevealed((prev) => { - const next = new Set(prev); - if (next.has(id)) next.delete(id); - else next.add(id); - return next; - }); - } - return ( @@ -115,24 +105,8 @@ export function CredentialsTab({ taskId }: { taskId: string }) {
- {c.password === "" ? "— agent generates —" : revealed.has(c.id) ? c.password : "•".repeat(Math.min(c.password.length, 12))} + {c.password === "" ? "— agent generates —" : "stored, hidden"} - {c.password !== "" ? ( - <> - - - - ) : null}