refactor: 代码审阅优化

P0: - fix(config): config_dir 使用 home_dir 回退，避免 ~ 不展开 - fix(search): 模糊查询转义 LIKE 通配符 % 和 _ P1: - chore(db): 连接池添加 acquire_timeout 10s - refactor(update): 消除 meta_keys/secret_keys 重复计算 P2: - refactor(config): 合并 ConfigAction 枚举 - chore(deps): 移除 clap/env、uuid/v4 无用 features - perf(main): delete 命令跳过 master_key 加载 - i18n(config): 统一错误消息为英文 - perf(search): show_secrets=false 时不再解密获取 key_count - feat(delete,update): 支持 -o json/json-compact 输出 P3: - feat(search): --tag 支持多值交叉过滤 docs: 将 seed-data.sh 替换为 setup-gitea-actions.sh Made-with: Cursor
2026-03-19 09:31:53 +08:00
parent dc0534cbc9
commit 31b0ea9bf1
11 changed files with 189 additions and 126 deletions
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -22,7 +22,7 @@ secrets/
      delete.rs          # delete 命令
      update.rs          # update 命令：增量更新（合并 tags/metadata/encrypted）
  scripts/
-    seed-data.sh         # 从 refining/ricnsmart config.toml 导入全量数据
+    setup-gitea-actions.sh  # 配置 Gitea Actions 变量与 Secrets
  .gitea/workflows/
    secrets.yml          # CI：fmt + clippy + musl 构建 + Release 上传 + 飞书通知
  .vscode/tasks.json     # 本地测试任务（build / config / search / add+delete / update / audit 等）
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -2229,7 +2229,6 @@ version = "1.22.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "a68d3c8f01c0cfa54a75291d83601161799e4a89a39e0929f4b0354d88757a37"
 dependencies = [
- "getrandom 0.4.2",
 "js-sys",
 "serde_core",
 "wasm-bindgen",
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -8,7 +8,7 @@ aes-gcm = "0.10.3"
 anyhow = "1.0.102"
 argon2 = { version = "0.5.3", features = ["std"] }
 chrono = { version = "0.4.44", features = ["serde"] }
-clap = { version = "4.6.0", features = ["derive", "env"] }
+clap = { version = "4.6.0", features = ["derive"] }
 dirs = "6.0.0"
 keyring = { version = "3.6.3", features = ["apple-native", "windows-native", "linux-native"] }
 rand = "0.10.0"
@@ -20,4 +20,4 @@ tokio = { version = "1.50.0", features = ["full"] }
 toml = "1.0.7"
 tracing = "0.1"
 tracing-subscriber = { version = "0.3", features = ["env-filter"] }
-uuid = { version = "1.22.0", features = ["serde", "v4"] }
+uuid = { version = "1.22.0", features = ["serde"] }
--- a/README.md
+++ b/README.md
@@ -186,7 +186,7 @@ src/
    delete.rs      # 删除
    update.rs      # 增量更新（合并 tags/metadata/encrypted）
 scripts/
-  seed-data.sh     # 导入 refining / ricnsmart 全量数据
+  setup-gitea-actions.sh  # 配置 Gitea Actions 变量与 Secrets
 ```

 ## CI/CD（Gitea Actions）
--- a/src/commands/config.rs
+++ b/src/commands/config.rs
@@ -1,43 +1,37 @@
 use crate::config::{self, Config, config_path};
 use anyhow::Result;

-pub enum ConfigAction {
-    SetDb { url: String },
-    Show,
-    Path,
-}
-
-pub async fn run(action: ConfigAction) -> Result<()> {
+pub async fn run(action: crate::ConfigAction) -> Result<()> {
    match action {
-        ConfigAction::SetDb { url } => {
+        crate::ConfigAction::SetDb { url } => {
            let cfg = Config {
                database_url: Some(url.clone()),
            };
            config::save_config(&cfg)?;
-            println!("✓ 数据库连接串已保存到: {}", config_path().display());
+            println!("Database URL saved to: {}", config_path().display());
            println!("  {}", mask_password(&url));
        }
-        ConfigAction::Show => {
+        crate::ConfigAction::Show => {
            let cfg = config::load_config()?;
            match cfg.database_url {
                Some(url) => {
                    println!("database_url = {}", mask_password(&url));
-                    println!("配置文件: {}", config_path().display());
+                    println!("config file:  {}", config_path().display());
                }
                None => {
-                    println!("未配置数据库连接串。");
-                    println!("请运行：secrets config set-db <DATABASE_URL>");
+                    println!("Database URL not configured.");
+                    println!("Run: secrets config set-db <DATABASE_URL>");
                }
            }
        }
-        ConfigAction::Path => {
+        crate::ConfigAction::Path => {
            println!("{}", config_path().display());
        }
    }
    Ok(())
 }

-/// 将 postgres://user:password@host/db 中的密码替换为 ***
+/// Mask the password in a postgres://user:password@host/db URL.
 fn mask_password(url: &str) -> String {
    if let Some(at_pos) = url.rfind('@')
        && let Some(scheme_end) = url.find("://")
--- a/src/commands/delete.rs
+++ b/src/commands/delete.rs
@@ -2,7 +2,15 @@ use anyhow::Result;
 use serde_json::json;
 use sqlx::PgPool;

-pub async fn run(pool: &PgPool, namespace: &str, kind: &str, name: &str) -> Result<()> {
+use crate::output::OutputMode;
+
+pub async fn run(
+    pool: &PgPool,
+    namespace: &str,
+    kind: &str,
+    name: &str,
+    output: OutputMode,
+) -> Result<()> {
    tracing::debug!(namespace, kind, name, "deleting record");

    let result =
@@ -15,10 +23,38 @@ pub async fn run(pool: &PgPool, namespace: &str, kind: &str, name: &str) -> Resu

    if result.rows_affected() == 0 {
        tracing::warn!(namespace, kind, name, "record not found for deletion");
-        println!("Not found: [{}/{}] {}", namespace, kind, name);
+        match output {
+            OutputMode::Json => println!(
+                "{}",
+                serde_json::to_string_pretty(
+                    &json!({"action":"not_found","namespace":namespace,"kind":kind,"name":name})
+                )?
+            ),
+            OutputMode::JsonCompact => println!(
+                "{}",
+                serde_json::to_string(
+                    &json!({"action":"not_found","namespace":namespace,"kind":kind,"name":name})
+                )?
+            ),
+            _ => println!("Not found: [{}/{}] {}", namespace, kind, name),
+        }
    } else {
        crate::audit::log(pool, "delete", namespace, kind, name, json!({})).await;
-        println!("Deleted: [{}/{}] {}", namespace, kind, name);
+        match output {
+            OutputMode::Json => println!(
+                "{}",
+                serde_json::to_string_pretty(
+                    &json!({"action":"deleted","namespace":namespace,"kind":kind,"name":name})
+                )?
+            ),
+            OutputMode::JsonCompact => println!(
+                "{}",
+                serde_json::to_string(
+                    &json!({"action":"deleted","namespace":namespace,"kind":kind,"name":name})
+                )?
+            ),
+            _ => println!("Deleted: [{}/{}] {}", namespace, kind, name),
+        }
    }
    Ok(())
 }
--- a/src/commands/search.rs
+++ b/src/commands/search.rs
@@ -10,7 +10,7 @@ pub struct SearchArgs<'a> {
    pub namespace: Option<&'a str>,
    pub kind: Option<&'a str>,
    pub name: Option<&'a str>,
-    pub tag: Option<&'a str>,
+    pub tags: &'a [String],
    pub query: Option<&'a str>,
    pub show_secrets: bool,
    pub fields: &'a [String],
@@ -37,13 +37,22 @@ pub async fn run(pool: &PgPool, args: SearchArgs<'_>, master_key: Option<&[u8; 3
        conditions.push(format!("name = ${}", idx));
        idx += 1;
    }
-    if args.tag.is_some() {
-        conditions.push(format!("tags @> ARRAY[${}]", idx));
+    if !args.tags.is_empty() {
+        // Use PostgreSQL array containment: tags @> ARRAY[$n, $m, ...] means all specified tags must be present
+        let placeholders: Vec<String> = args
+            .tags
+            .iter()
+            .map(|_| {
+                let p = format!("${}", idx);
                idx += 1;
+                p
+            })
+            .collect();
+        conditions.push(format!("tags @> ARRAY[{}]", placeholders.join(", ")));
    }
    if args.query.is_some() {
        conditions.push(format!(
-            "(name ILIKE ${i} OR namespace ILIKE ${i} OR kind ILIKE ${i} OR metadata::text ILIKE ${i} OR EXISTS (SELECT 1 FROM unnest(tags) t WHERE t ILIKE ${i}))",
+            "(name ILIKE ${i} ESCAPE '\\' OR namespace ILIKE ${i} ESCAPE '\\' OR kind ILIKE ${i} ESCAPE '\\' OR metadata::text ILIKE ${i} ESCAPE '\\' OR EXISTS (SELECT 1 FROM unnest(tags) t WHERE t ILIKE ${i} ESCAPE '\\'))",
            i = idx
        ));
        idx += 1;
@@ -81,11 +90,16 @@ pub async fn run(pool: &PgPool, args: SearchArgs<'_>, master_key: Option<&[u8; 3
    if let Some(v) = args.name {
        q = q.bind(v);
    }
-    if let Some(v) = args.tag {
-        q = q.bind(v);
+    for v in args.tags {
+        q = q.bind(v.as_str());
    }
    if let Some(v) = args.query {
-        q = q.bind(format!("%{}%", v));
+        q = q.bind(format!(
+            "%{}%",
+            v.replace('\\', "\\\\")
+                .replace('%', "\\%")
+                .replace('_', "\\_")
+        ));
    }
    q = q.bind(args.limit as i64).bind(args.offset as i64);

@@ -186,7 +200,7 @@ fn to_json(
            Err(e) => json!({"_error": e.to_string()}),
        }
    } else {
-        json!({"_encrypted": true, "_key_count": encrypted_key_count(row, master_key)})
+        json!({"_encrypted": true})
    };

    json!({
@@ -201,19 +215,6 @@ fn to_json(
        "updated_at": row.updated_at.format("%Y-%m-%dT%H:%M:%SZ").to_string(),
    })
 }
-
-/// Return the number of keys in the encrypted JSON (decrypts to count; 0 on failure).
-fn encrypted_key_count(row: &Secret, master_key: Option<&[u8; 32]>) -> usize {
-    if row.encrypted.is_empty() {
-        return 0;
-    }
-    let Some(key) = master_key else { return 0 };
-    match crypto::decrypt_json(key, &row.encrypted) {
-        Ok(Value::Object(m)) => m.len(),
-        _ => 0,
-    }
-}
-
 fn print_text(
    row: &Secret,
    show_secrets: bool,
--- a/src/commands/update.rs
+++ b/src/commands/update.rs
@@ -5,6 +5,7 @@ use uuid::Uuid;

 use super::add::parse_kv;
 use crate::crypto;
+use crate::output::OutputMode;

 #[derive(FromRow)]
 struct UpdateRow {
@@ -24,6 +25,7 @@ pub struct UpdateArgs<'a> {
    pub remove_meta: &'a [String],
    pub secret_entries: &'a [String],
    pub remove_secrets: &'a [String],
+    pub output: OutputMode,
 }

 pub async fn run(pool: &PgPool, args: UpdateArgs<'_>, master_key: &[u8; 32]) -> Result<()> {
@@ -141,8 +143,28 @@ pub async fn run(pool: &PgPool, args: UpdateArgs<'_>, master_key: &[u8; 32]) ->
    )
    .await;

-    println!("Updated: [{}/{}] {}", args.namespace, args.kind, args.name);
+    let result_json = json!({
+        "action": "updated",
+        "namespace": args.namespace,
+        "kind": args.kind,
+        "name": args.name,
+        "add_tags": args.add_tags,
+        "remove_tags": args.remove_tags,
+        "meta_keys": meta_keys,
+        "remove_meta": args.remove_meta,
+        "secret_keys": secret_keys,
+        "remove_secrets": args.remove_secrets,
+    });

+    match args.output {
+        OutputMode::Json => {
+            println!("{}", serde_json::to_string_pretty(&result_json)?);
+        }
+        OutputMode::JsonCompact => {
+            println!("{}", serde_json::to_string(&result_json)?);
+        }
+        _ => {
+            println!("Updated: [{}/{}] {}", args.namespace, args.kind, args.name);
            if !args.add_tags.is_empty() {
                println!("  +tags:     {}", args.add_tags.join(", "));
            }
@@ -150,27 +172,19 @@ pub async fn run(pool: &PgPool, args: UpdateArgs<'_>, master_key: &[u8; 32]) ->
                println!("  -tags:     {}", args.remove_tags.join(", "));
            }
            if !args.meta_entries.is_empty() {
-        let keys: Vec<&str> = args
-            .meta_entries
-            .iter()
-            .filter_map(|s| s.split_once('=').map(|(k, _)| k))
-            .collect();
-        println!("  +metadata: {}", keys.join(", "));
+                println!("  +metadata: {}", meta_keys.join(", "));
            }
            if !args.remove_meta.is_empty() {
                println!("  -metadata: {}", args.remove_meta.join(", "));
            }
            if !args.secret_entries.is_empty() {
-        let keys: Vec<&str> = args
-            .secret_entries
-            .iter()
-            .filter_map(|s| s.split_once('=').map(|(k, _)| k))
-            .collect();
-        println!("  +secrets:  {}", keys.join(", "));
+                println!("  +secrets:  {}", secret_keys.join(", "));
            }
            if !args.remove_secrets.is_empty() {
                println!("  -secrets:  {}", args.remove_secrets.join(", "));
            }
+        }
+    }

    Ok(())
 }
--- a/src/config.rs
+++ b/src/config.rs
@@ -10,7 +10,8 @@ pub struct Config {

 pub fn config_dir() -> PathBuf {
    dirs::config_dir()
-        .unwrap_or_else(|| PathBuf::from("~/.config"))
+        .or_else(|| dirs::home_dir().map(|h| h.join(".config")))
+        .unwrap_or_else(|| PathBuf::from(".config"))
        .join("secrets")
 }

@@ -24,36 +25,38 @@ pub fn load_config() -> Result<Config> {
        return Ok(Config::default());
    }
    let content = fs::read_to_string(&path)
-        .with_context(|| format!("读取配置文件失败: {}", path.display()))?;
+        .with_context(|| format!("failed to read config file: {}", path.display()))?;
    let config: Config = toml::from_str(&content)
-        .with_context(|| format!("解析配置文件失败: {}", path.display()))?;
+        .with_context(|| format!("failed to parse config file: {}", path.display()))?;
    Ok(config)
 }

 pub fn save_config(config: &Config) -> Result<()> {
    let dir = config_dir();
-    fs::create_dir_all(&dir).with_context(|| format!("创建配置目录失败: {}", dir.display()))?;
+    fs::create_dir_all(&dir)
+        .with_context(|| format!("failed to create config dir: {}", dir.display()))?;

    let path = config_path();
-    let content = toml::to_string_pretty(config).context("序列化配置失败")?;
-    fs::write(&path, &content).with_context(|| format!("写入配置文件失败: {}", path.display()))?;
+    let content = toml::to_string_pretty(config).context("failed to serialize config")?;
+    fs::write(&path, &content)
+        .with_context(|| format!("failed to write config file: {}", path.display()))?;

-    // 设置文件权限为 0600（仅所有者读写）
+    // Set file permissions to 0600 (owner read/write only)
    #[cfg(unix)]
    {
        use std::os::unix::fs::PermissionsExt;
        let perms = fs::Permissions::from_mode(0o600);
        fs::set_permissions(&path, perms)
-            .with_context(|| format!("设置文件权限失败: {}", path.display()))?;
+            .with_context(|| format!("failed to set file permissions: {}", path.display()))?;
    }

    Ok(())
 }

-/// 按优先级解析数据库连接串：
-/// 1. --db-url CLI 参数（非空时使用）
-/// 2. ~/.config/secrets/config.toml 中的 database_url
-/// 3. 报错并提示用户配置
+/// Resolve database URL by priority:
+/// 1. --db-url CLI flag (if non-empty)
+/// 2. database_url in ~/.config/secrets/config.toml
+/// 3. Error with setup instructions
 pub fn resolve_db_url(cli_db_url: &str) -> Result<String> {
    if !cli_db_url.is_empty() {
        return Ok(cli_db_url.to_string());
@@ -66,5 +69,5 @@ pub fn resolve_db_url(cli_db_url: &str) -> Result<String> {
        return Ok(url);
    }

-    anyhow::bail!("数据库未配置。请先运行：\n\n    secrets config set-db <DATABASE_URL>\n")
+    anyhow::bail!("Database not configured. Run:\n\n    secrets config set-db <DATABASE_URL>\n")
 }
--- a/src/db.rs
+++ b/src/db.rs
@@ -6,6 +6,7 @@ pub async fn create_pool(database_url: &str) -> Result<PgPool> {
    tracing::debug!("connecting to database");
    let pool = PgPoolOptions::new()
        .max_connections(5)
+        .acquire_timeout(std::time::Duration::from_secs(10))
        .connect(database_url)
        .await?;
    tracing::debug!("database connection established");
--- a/src/main.rs
+++ b/src/main.rs
@@ -18,7 +18,10 @@ use output::resolve_output_mode;
    version,
    about = "Secrets & config manager backed by PostgreSQL — optimised for AI agents",
    after_help = "QUICK START:
-  # First time setup (run once per device)
+  # 1. Configure database (once per device)
+  secrets config set-db \"postgres://postgres:<password>@<host>:<port>/secrets\"
+
+  # 2. Initialize master key (once per device)
  secrets init

  # Discover what namespaces / kinds exist
@@ -52,7 +55,12 @@ enum Commands {
    ///
    /// Prompts for a master password, derives a key with Argon2id, and stores
    /// it in the OS Keychain. Use the same password on every device.
-    #[command(after_help = "EXAMPLES:
+    ///
+    /// NOTE: Run `secrets config set-db <URL>` first if database is not configured.
+    #[command(after_help = "PREREQUISITE:
+  Database must be configured first. Run: secrets config set-db <DATABASE_URL>
+
+EXAMPLES:
  # First device: generates a new Argon2id salt and stores master key
  secrets init

@@ -155,9 +163,9 @@ enum Commands {
        /// Exact name filter, e.g. gitea, i-uf63f2uookgs5uxmrdyc
        #[arg(long)]
        name: Option<String>,
-        /// Filter by tag, e.g. --tag aliyun
+        /// Filter by tag, e.g. --tag aliyun (repeatable for AND intersection)
        #[arg(long)]
-        tag: Option<String>,
+        tag: Vec<String>,
        /// Fuzzy keyword (matches name, namespace, kind, tags, metadata text)
        #[arg(short, long)]
        query: Option<String>,
@@ -201,6 +209,9 @@ enum Commands {
        /// Exact name of the record to delete
        #[arg(long)]
        name: String,
+        /// Output format: text (default on TTY), json, json-compact
+        #[arg(short, long = "output")]
+        output: Option<String>,
    },

    /// Incrementally update an existing record (merge semantics; record must exist).
@@ -252,6 +263,9 @@ enum Commands {
        /// Delete a secret field by key (repeatable)
        #[arg(long = "remove-secret")]
        remove_secrets: Vec<String>,
+        /// Output format: text (default on TTY), json, json-compact
+        #[arg(short, long = "output")]
+        output: Option<String>,
    },

    /// Manage CLI configuration (database connection, etc.)
@@ -298,15 +312,8 @@ async fn main() -> Result<()> {
        .init();

    // config subcommand needs no database or master key
-    if let Commands::Config { action } = &cli.command {
-        let cmd_action = match action {
-            ConfigAction::SetDb { url } => {
-                commands::config::ConfigAction::SetDb { url: url.clone() }
-            }
-            ConfigAction::Show => commands::config::ConfigAction::Show,
-            ConfigAction::Path => commands::config::ConfigAction::Path,
-        };
-        return commands::config::run(cmd_action).await;
+    if let Commands::Config { action } = cli.command {
+        return commands::config::run(action).await;
    }

    let db_url = config::resolve_db_url(&cli.db_url)?;
@@ -314,14 +321,14 @@ async fn main() -> Result<()> {
    db::migrate(&pool).await?;

    // init needs a pool but sets up the master key — handle before loading it
-    if let Commands::Init = &cli.command {
+    if let Commands::Init = cli.command {
        return commands::init::run(&pool).await;
    }

-    // All remaining commands require the master key from the OS Keychain
-    let master_key = crypto::load_master_key()?;
+    // All remaining commands require the master key from the OS Keychain,
+    // except delete which operates on plaintext metadata only.

-    match &cli.command {
+    match cli.command {
        Commands::Init | Commands::Config { .. } => unreachable!(),

        Commands::Add {
@@ -333,18 +340,19 @@ async fn main() -> Result<()> {
            secrets,
            output,
        } => {
+            let master_key = crypto::load_master_key()?;
            let _span =
                tracing::info_span!("cmd", command = "add", %namespace, %kind, %name).entered();
            let out = resolve_output_mode(output.as_deref())?;
            commands::add::run(
                &pool,
                commands::add::AddArgs {
-                    namespace,
-                    kind,
-                    name,
-                    tags,
-                    meta_entries: meta,
-                    secret_entries: secrets,
+                    namespace: &namespace,
+                    kind: &kind,
+                    name: &name,
+                    tags: &tags,
+                    meta_entries: &meta,
+                    secret_entries: &secrets,
                    output: out,
                },
                &master_key,
@@ -366,8 +374,9 @@ async fn main() -> Result<()> {
            sort,
            output,
        } => {
+            let master_key = crypto::load_master_key()?;
            let _span = tracing::info_span!("cmd", command = "search").entered();
-            let show = *show_secrets || fields.iter().any(|f| f.starts_with("secret"));
+            let show = show_secrets || fields.iter().any(|f| f.starts_with("secret"));
            let out = resolve_output_mode(output.as_deref())?;
            commands::search::run(
                &pool,
@@ -375,14 +384,14 @@ async fn main() -> Result<()> {
                    namespace: namespace.as_deref(),
                    kind: kind.as_deref(),
                    name: name.as_deref(),
-                    tag: tag.as_deref(),
+                    tags: &tag,
                    query: query.as_deref(),
                    show_secrets: show,
-                    fields,
-                    summary: *summary,
-                    limit: *limit,
-                    offset: *offset,
-                    sort,
+                    fields: &fields,
+                    summary,
+                    limit,
+                    offset,
+                    sort: &sort,
                    output: out,
                },
                Some(&master_key),
@@ -394,10 +403,12 @@ async fn main() -> Result<()> {
            namespace,
            kind,
            name,
+            output,
        } => {
            let _span =
                tracing::info_span!("cmd", command = "delete", %namespace, %kind, %name).entered();
-            commands::delete::run(&pool, namespace, kind, name).await?;
+            let out = resolve_output_mode(output.as_deref())?;
+            commands::delete::run(&pool, &namespace, &kind, &name, out).await?;
        }

        Commands::Update {
@@ -410,21 +421,25 @@ async fn main() -> Result<()> {
            remove_meta,
            secrets,
            remove_secrets,
+            output,
        } => {
+            let master_key = crypto::load_master_key()?;
            let _span =
                tracing::info_span!("cmd", command = "update", %namespace, %kind, %name).entered();
+            let out = resolve_output_mode(output.as_deref())?;
            commands::update::run(
                &pool,
                commands::update::UpdateArgs {
-                    namespace,
-                    kind,
-                    name,
-                    add_tags,
-                    remove_tags,
-                    meta_entries: meta,
-                    remove_meta,
-                    secret_entries: secrets,
-                    remove_secrets,
+                    namespace: &namespace,
+                    kind: &kind,
+                    name: &name,
+                    add_tags: &add_tags,
+                    remove_tags: &remove_tags,
+                    meta_entries: &meta,
+                    remove_meta: &remove_meta,
+                    secret_entries: &secrets,
+                    remove_secrets: &remove_secrets,
+                    output: out,
                },
                &master_key,
            )