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..1266654 --- /dev/null +++ b/server/secrets_api_test.go @@ -0,0 +1,146 @@ +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") + } +} + +// 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 3a378df..c236627 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,80 @@ 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. +// 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, storedEnv 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 + } + var stored *db.MCPServer + for _, cur := range all { + if m.ID != 0 && cur.ID == m.ID { + 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 := 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 + } + } + m.Env, err = json.Marshal(env) + 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 { @@ -871,6 +943,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()) 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}