From ec388f772062872a5b29e79e2e7c8bb94fa0acb1 Mon Sep 17 00:00:00 2001 From: Cooper Maruyama Date: Wed, 10 Jun 2026 08:15:10 -0700 Subject: [PATCH 1/6] refactor: deepen seams from architecture review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Land the 2026-06-09 review (hm-x9r, hm-i9k, hm-y36, hm-x7z, hm-cpc, hm-dwr, hm-a16): - Context owns project-config resolution (ctx.project_config(), memoized) — fixes five sites that ignored --project (generate, codegen x2, export, check, TUI health) - OutputResolver: one decrypt-once scan behind open(); tri-state env_map makes exec's label path a one-liner; generate/codegen stop decrypting the store twice - exec accepts multiple refs (union; conflicting values hard-error) - ADR-0001: keep last-wins collapse in file generators; codegen gains generate's duplicate-key warning - StoreOps owns the mutation chain (commit/push/completions refresh) for both the CLI dispatcher and TUI cores — TUI delete/rekey no longer leave the store dirty; join no longer prints into ratatui - KeyRegistry: exhaustive-match rows derive dispatch, live help, and palette shortcuts; envs config key renamed outputs (serde alias) - PathFolding/ResultSort/StoreHealth graduate out of the search view as terminal-free modules Also fixes: completions cache missed same-second deletions (mtime fingerprint now nanos + entry count); Ctrl+Space chords could not roundtrip the config format; viewer rekey targeted the ambient store instead of the secret's own. Gates: 691 lib + 155 integration tests, clippy clean. Co-Authored-By: Claude Fable 5 --- CONTEXT.md | 8 + README.md | 17 +- docs/ARCHITECTURE.md | 5 + ...p-last-wins-collapse-in-file-generators.md | 46 ++ rust/src/cli/check.rs | 4 +- rust/src/cli/codegen.rs | 82 +-- rust/src/cli/completions.rs | 9 +- rust/src/cli/doctor.rs | 1 + rust/src/cli/exec.rs | 258 +++----- rust/src/cli/export.rs | 4 +- rust/src/cli/generate.rs | 78 +-- rust/src/cli/get.rs | 27 +- rust/src/cli/import.rs | 3 + rust/src/cli/init.rs | 1 + rust/src/cli/join.rs | 44 +- rust/src/cli/mod.rs | 134 ++-- rust/src/cli/output_resolver.rs | 570 +++++++++++++++++ rust/src/cli/recipient.rs | 50 +- rust/src/cli/resolver.rs | 26 - rust/src/cli/schema.rs | 1 + rust/src/cli/search.rs | 2 +- rust/src/cli/set.rs | 2 +- rust/src/cli/store_ops.rs | 273 ++++++++ rust/src/cli/sync.rs | 1 + rust/src/completions_cache.rs | 70 ++- rust/src/config/mod.rs | 21 +- rust/src/tui/app.rs | 26 +- rust/src/tui/harness.rs | 1 + rust/src/tui/keymap.rs | 426 ++++++++++--- rust/src/tui/mod.rs | 4 + rust/src/tui/model/mod.rs | 8 + rust/src/tui/model/path_folding.rs | 279 +++++++++ rust/src/tui/model/result_sort.rs | 194 ++++++ rust/src/tui/views/command_palette.rs | 32 +- rust/src/tui/views/help.rs | 31 +- rust/src/tui/views/new_secret.rs | 71 +-- rust/src/tui/views/outputs.rs | 2 + rust/src/tui/views/recipient_add.rs | 5 +- rust/src/tui/views/recipient_list.rs | 13 +- rust/src/tui/views/remote_add.rs | 1 + rust/src/tui/views/search.rs | 589 ++---------------- rust/src/tui/views/secret_viewer.rs | 68 +- rust/src/tui/widgets/mod.rs | 1 + rust/src/tui/widgets/store_health.rs | 197 ++++++ tests/integration/cli_test.rs | 307 ++++++++- 45 files changed, 2833 insertions(+), 1159 deletions(-) create mode 100644 docs/adr/0001-keep-last-wins-collapse-in-file-generators.md create mode 100644 rust/src/cli/output_resolver.rs create mode 100644 rust/src/cli/store_ops.rs create mode 100644 rust/src/tui/model/mod.rs create mode 100644 rust/src/tui/model/path_folding.rs create mode 100644 rust/src/tui/model/result_sort.rs create mode 100644 rust/src/tui/widgets/store_health.rs diff --git a/CONTEXT.md b/CONTEXT.md index c79077a..526e83f 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -20,3 +20,11 @@ These name the deepened modules introduced by the architecture review. - **GitAdapter** — the seam for git operations. Production: `CliGitAdapter` (shells out). Tests: `InMemoryGitAdapter`. Absorbs the commit/push/pull orchestration that was previously inline in Context. - **SecretStore** — the deepened `remote::store` module. A struct that owns the store root and resolves `recipients_path` once at construction. Narrow interface: `read`, `write`, `list`, `recipients`. - **SecretResolver** — the deepened secret-resolution pipeline. One module owns the full path from reference string to decrypted `DecodedSecret`. Absorbs the duplicated ref→store→decrypt→decode pipeline that was spread across 5 CLI modules. + +## Architecture Terms (from 2026-06-09 review) + +- **Context · project config** — Context owns "which project config applies to this invocation": `ctx.project_config()`, lazy and memoized, selecting `--project` root over cwd walk internally. The raw loaders are private to `config`; `migrate`'s multi-root scan is the one sanctioned direct caller of the explicit-root loader. The legacy-`envs:` warning firing at most once per process is a property of this module, not of call-site discipline. +- **OutputResolver** — the deepened Output pipeline: project `outputs:` map → candidates with decrypted tags → selector/alias resolution → decoded entries, consumed by exec, generate, codegen (and later the TUI outputs view). `open(ctx)` performs the single decrypt-once scan (zero I/O when no outputs are defined); `env_map(label, tags)` is exec's tri-state strict surface (local-store only, tag-filter before env-key-conflict check, conflict = hard error); `decode(output)` follows cross-store entries and preserves duplicate keys for caller-side collapse. Materialization is channel-free — `DecodedEntry` carries expiry, callers choose where warnings render. Absorbs `resolver_candidates_with_tags`, exec's inline candidate loop, and `SecretResolver::resolve_candidates`. +- **KeyRegistry** — the deepened TUI keybinding module: one exhaustive-match row per `KeyAction` (a missing row is a compile error) owning the config-field accessor, help text, view scope, and palette link. `KeyMap::entries()`, view help screens (rendered from the *live* KeyMap, so rebinds show up), and palette shortcut display all derive from the registry; non-rebindable navigation rows (arrows/esc) stay static per view. The serde `KeyMap` struct remains the user-facing config format; the legacy `envs` field is renamed `outputs` with a serde alias. +- **StoreOps** — the deepened mutation seam between presentation and the Store: one central module of silent mutation cores (set, delete, rekey, join, recipient add/rm, remote add). Each core owns the full side-effect chain — validate → encrypt → write → commit → completions-cache refresh — with no stdin/stdout. CLI commands are presentation wrappers; TUI views call the same cores, so the two fronts cannot drift. Generalizes the `add_core`/`rm_core` precedent from hm-by7. +- **PathFolding · ResultSort · StoreHealth** — three modules graduated from the Search view (`tui/model/path_folding.rs`, `tui/model/result_sort.rs`, `tui/widgets/store_health.rs`), following the autocomplete-widget precedent. PathFolding (store-bucket partitioning + collapse/expand of Secret paths to prefix groups) and ResultSort (column + direction ordering) are pure state modules — results in, rows out, no ratatui imports — unit-tested without a terminal. StoreHealth is a drawable widget owning health fetch + pill rendering; its project-config read goes through Context · project config. The Outputs view was evaluated for adoption and deliberately left out: its list is a one-line name sort, so sharing the column-sort machinery would have been a hypothetical seam. diff --git a/README.md b/README.md index 0edd209..4f339df 100644 --- a/README.md +++ b/README.md @@ -479,9 +479,10 @@ himitsu tag prod/STRIPE_KEY rm rotate-2026-q1 absent tags. The grammar `[A-Za-z0-9_.-]+` (1-64 chars, case-sensitive) is validated up front for both `add` and `rm`. -### `himitsu exec -- ...` +### `himitsu exec ... -- ...` -Run a command with secrets injected as environment variables. `` is +Run a command with secrets injected as environment variables. One or more +refs may be passed; the union of all refs is injected. Each `` is one of: 1. **Output label** from project config `outputs:` (e.g. `pci-prod`) -- uses @@ -501,13 +502,16 @@ himitsu exec pci-prod -- node app.js himitsu exec tag:pci+tag:prod -- node app.js himitsu exec prod/*+tag:pci -- ./run-checks.sh himitsu exec prod/API_KEY -i -- env | grep API_KEY # `-i` = clean env +himitsu exec pci-prod common/DB_URL -- node app.js # union of two refs ``` The variable name for each secret comes from (in priority order): the output alias key → `SecretValue.env_key` → `derive_env_key(path tail)` (e.g. `api-key` → `API_KEY`, `group/item-name` → `GROUP__ITEM_NAME`). Two secrets resolving to the same env-var name is a hard error naming both -source paths. +source paths. With multiple refs, every ref must match at least one secret, +and an env var resolving to *different* values via different refs is a hard +error (the same secret reached by overlapping refs is fine). If a selector matches no secrets, `himitsu exec` exits 1 with `error: selector 'X' matched no secrets`. @@ -840,9 +844,10 @@ binding immediately. The full action list (with defaults) is in [`rust/src/tui/keymap.rs`](rust/src/tui/keymap.rs): `quit`, `help`, `command_palette`, `new_secret`, `switch_store`, -`copy_selected`, `copy_ref_selected`, `envs`, `reveal`, `copy_value`, -`copy_ref`, `rekey`, `edit`, `delete`, `back`, `save_secret`, -`next_field`, `prev_field`, `cancel`. +`copy_selected`, `copy_ref_selected`, `outputs` (legacy alias: `envs`), +`collapse_paths`, `expand_paths`, `toggle_autocomplete`, `refine_tag`, +`sort_column`, `reveal`, `copy_value`, `copy_ref`, `rekey`, `edit`, +`delete`, `back`, `save_secret`, `next_field`, `prev_field`, `cancel`. ## Sync & Store Health diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index a8ef570..68c7c64 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -367,6 +367,11 @@ filesystem paths). Conflicts (two secrets resolving to the same env-var name) are a hard error: a half-injected environment is more confusing than a clear failure. +Multiple refs may be passed (`himitsu exec tag:pci prod/* -- cmd`); the +union is injected. Each ref must match at least one secret, and an env var +resolving to different values via different refs is a hard error (the same +secret reached by overlapping refs is tolerated). + Tab completion for `exec` uses fuzzy subsequence matching via `nucleo_matcher` (`--fuzzy` flag on `__complete-paths`); other subcommands retain exact-prefix matching. diff --git a/docs/adr/0001-keep-last-wins-collapse-in-file-generators.md b/docs/adr/0001-keep-last-wins-collapse-in-file-generators.md new file mode 100644 index 0000000..aa5d3bf --- /dev/null +++ b/docs/adr/0001-keep-last-wins-collapse-in-file-generators.md @@ -0,0 +1,46 @@ +# ADR-0001: Keep last-wins duplicate-key collapse in file generators + +Date: 2026-06-10 +Status: accepted + +## Context + +When an Output resolves two entries to the same env-var key, the three +consumers historically disagreed: + +- `exec` hard-errors (a half-injected environment is forbidden); +- `generate` warns ("duplicate key … — using last value") and takes the + last value; +- `codegen` (sops mode) silently took the last value. + +The 2026-06-09 architecture review (OutputResolver, hm-i9k) made collapse +policy explicitly caller-owned — `OutputResolver::decode` preserves +duplicate keys in entry order — and deferred the question of unifying on a +hard error to hm-x7z. + +## Decision + +Keep last-wins collapse in the file generators (`generate`, `codegen` sops +mode). Bring `codegen` to warning parity with `generate` (the silent +variant now warns). `exec` keeps its hard error. Do not harden the file +generators to hard errors. + +## Rationale + +- Within a resolved Output, alias entries resolve **after** selector + entries (`resolve_output_def`: selectors first, then aliases). Last-wins + therefore means "the alias pins this binding" — a deliberate, useful + override pattern: selectors sweep broadly, aliases pin specific keys. + Hard-erroring would break configs using that pattern. +- Generated files are reviewable artifacts (sops YAML in PRs); the + dangerous surface is `exec`'s injected environment, which already + hard-errors on conflicts. +- The remaining gap was `codegen`'s *silent* clobber — fixed by warning + parity, which is informational and non-breaking. + +## Consequences + +- Duplicate keys in `generate` and `codegen` warn on stderr and take the + last value; `exec` remains strict. +- Future architecture reviews should not re-propose hardening the file + generators without revisiting this ADR. diff --git a/rust/src/cli/check.rs b/rust/src/cli/check.rs index cb29f80..b2386d2 100644 --- a/rust/src/cli/check.rs +++ b/rust/src/cli/check.rs @@ -106,7 +106,7 @@ pub fn run(args: CheckArgs, ctx: &Context) -> Result<()> { /// 1. Explicit `args.store` slug. /// 2. Slugs referenced in global or project config. /// 3. All known stores (`list_remotes()`). -fn discover_stores(args: &CheckArgs, _ctx: &Context) -> Result> { +fn discover_stores(args: &CheckArgs, ctx: &Context) -> Result> { // 1. Explicit store argument if let Some(ref slug) = args.store { config::validate_remote_slug(slug)?; @@ -116,7 +116,7 @@ fn discover_stores(args: &CheckArgs, _ctx: &Context) -> Result> { // 2. Global + project config let global = config::Config::load(&config::config_path()).unwrap_or_default(); let mut slugs = collect_stores_from_global_config(&global); - if let Some((cfg, _path)) = config::load_project_config().ok().flatten() { + if let Some((cfg, _path)) = ctx.project_config().ok().flatten() { slugs.extend(collect_stores_from_project_config(&cfg)); } if !slugs.is_empty() { diff --git a/rust/src/cli/codegen.rs b/rust/src/cli/codegen.rs index 7c8d9bf..ed69c6c 100644 --- a/rust/src/cli/codegen.rs +++ b/rust/src/cli/codegen.rs @@ -6,10 +6,8 @@ use clap::Args; use tracing::{debug, info}; use super::Context; -use crate::config::outputs::resolver::{ - resolve_outputs, Context as ResolverContext, ResolvedOutput, -}; -use crate::config::{self, load_project_config, validate_env_label}; +use crate::config::outputs::resolver::ResolvedOutput; +use crate::config::validate_env_label; use crate::error::{HimitsuError, Result}; use crate::proto::{self, CodegenLang}; @@ -89,12 +87,11 @@ pub fn run(args: CodegenArgs, ctx: &Context) -> Result<()> { args.env, ); - // 2. Load project outputs config and resolve. - let outputs_map = load_project_config()? - .map(|(cfg, _)| cfg.outputs) - .unwrap_or_default(); + // 2. Resolve the project's outputs (candidates carry real decrypted tags + // so `tag:` selectors and selector-valued aliases match). + let outputs = super::output_resolver::OutputResolver::open(ctx)?; - if outputs_map.is_empty() { + if outputs.all().is_empty() { return Err(HimitsuError::InvalidConfig( "no `outputs` defined in project config — \ define outputs: blocks in himitsu.yaml" @@ -102,12 +99,8 @@ pub fn run(args: CodegenArgs, ctx: &Context) -> Result<()> { )); } - let available_secrets = super::resolver_candidates_with_tags(ctx); - let resolver_ctx = ResolverContext { available_secrets }; - let resolved_outputs = resolve_outputs(&outputs_map, &resolver_ctx)?; - // 3. Build inventory from resolved outputs. - let inventory = build_inventory_from_outputs(&resolved_outputs); + let inventory = build_inventory_from_outputs(outputs.all()); if inventory.all_keys.is_empty() { return Err(HimitsuError::InvalidConfig( @@ -150,11 +143,9 @@ pub fn run(args: CodegenArgs, ctx: &Context) -> Result<()> { fn run_sops(label: &str, output_override: Option<&str>, ctx: &Context) -> Result<()> { validate_env_label(label)?; - let outputs_map = load_project_config()? - .map(|(cfg, _)| cfg.outputs) - .unwrap_or_default(); + let outputs = super::output_resolver::OutputResolver::open(ctx)?; - if outputs_map.is_empty() { + if outputs.all().is_empty() { return Err(HimitsuError::InvalidConfig( "no `outputs` defined in project config — \ define outputs: blocks in himitsu.yaml" @@ -162,43 +153,25 @@ fn run_sops(label: &str, output_override: Option<&str>, ctx: &Context) -> Result )); } - let available_secrets = super::resolver_candidates_with_tags(ctx); - let resolver_ctx = ResolverContext { available_secrets }; - let all_outputs = resolve_outputs(&outputs_map, &resolver_ctx)?; - - let resolved = all_outputs - .into_iter() - .find(|o| o.name == label) + let resolved = outputs + .get(label) .ok_or_else(|| HimitsuError::InvalidConfig(format!("unknown output: {label}")))?; - let identities = ctx.load_identities()?; let mut output: BTreeMap = BTreeMap::new(); - for entry in &resolved.entries { - let effective_store = if let Some(ref slug) = entry.store_slug { - config::ensure_store(slug)? - } else { - ctx.store.clone() - }; - let payload = - crate::remote::store::read_secret_payload(&effective_store, &entry.secret_path)?; - let plaintext = - match crate::crypto::age::decrypt_with_identities(&payload.ciphertext, &identities) { - Ok(p) => p, - Err(_) if payload.legacy_proto_envelope => payload.ciphertext, - Err(err) => return Err(err), - }; - let decoded = crate::crypto::secret_value::decode_with_legacy_environment( - &plaintext, - payload.legacy_environment.as_deref(), - ); - super::get::warn_if_expired(&entry.secret_path, &decoded); - let value = String::from_utf8(decoded.data).map_err(|e| { - HimitsuError::DecryptionFailed(format!( - "non-UTF-8 secret at '{}': {e}", - entry.secret_path - )) - })?; - output.insert(entry.env_key.clone(), value); + for entry in outputs.decode(resolved)? { + if let Some(warning) = entry.expiry_warning() { + eprintln!("{warning}"); + } + // Last-wins collapse, warned: aliases resolve after selectors, so a + // duplicate key reads as "the alias pins this binding". See + // docs/adr/0001-keep-last-wins-collapse-in-file-generators.md. + if output.contains_key(&entry.env_key) { + eprintln!( + "warning: duplicate key '{}' in output '{label}' — using last value", + entry.env_key + ); + } + output.insert(entry.env_key, entry.value); } let body = serde_yaml::to_string(&output)?; @@ -1015,6 +988,7 @@ mod tests { key_provider: crate::config::KeyProvider::default(), project_root: None, git: std::sync::Arc::new(crate::git::CliGitAdapter), + project_config_cell: Default::default(), }; let args = CodegenArgs { @@ -1044,6 +1018,7 @@ mod tests { key_provider: crate::config::KeyProvider::default(), project_root: None, git: std::sync::Arc::new(crate::git::CliGitAdapter), + project_config_cell: Default::default(), }; let args = CodegenArgs { env_positional: None, @@ -1073,6 +1048,7 @@ mod tests { key_provider: crate::config::KeyProvider::default(), project_root: None, git: std::sync::Arc::new(crate::git::CliGitAdapter), + project_config_cell: Default::default(), }; let args = CodegenArgs { @@ -1106,6 +1082,7 @@ mod tests { key_provider: crate::config::KeyProvider::default(), project_root: None, git: std::sync::Arc::new(crate::git::CliGitAdapter), + project_config_cell: Default::default(), }; let args = CodegenArgs { @@ -1173,6 +1150,7 @@ mod tests { key_provider: crate::config::KeyProvider::default(), project_root: None, git: std::sync::Arc::new(crate::git::CliGitAdapter), + project_config_cell: Default::default(), }; let result = run_sops("ghost", None, &ctx); diff --git a/rust/src/cli/completions.rs b/rust/src/cli/completions.rs index 9bc9f59..ea19abf 100644 --- a/rust/src/cli/completions.rs +++ b/rust/src/cli/completions.rs @@ -286,8 +286,9 @@ fn patch_zsh(script: &str) -> String { let is_path_positional = (trimmed.starts_with("':path -- ") || trimmed.starts_with("'::path -- ")) && line.trim_end().ends_with(":_default' \\"); - let is_ref_positional = (trimmed.starts_with("':ref -- ") - || trimmed.starts_with("'::ref -- ")) + let is_ref_positional = (trimmed.starts_with("':refs -- ") + || trimmed.starts_with("'::refs -- ") + || trimmed.starts_with("'*::refs -- ")) && line.trim_end().ends_with(":_default' \\"); if is_path_positional { out.push_str(&line.replace(":_default'", ":_himitsu_secrets'")); @@ -450,8 +451,8 @@ mod tests { ); let ref_line = text .lines() - .find(|l| l.trim_start().starts_with("':ref -- ")) - .expect("exec ref positional present in generated zsh script"); + .find(|l| l.trim_start().starts_with("'*::refs -- ")) + .expect("exec refs positional present in generated zsh script"); assert!( ref_line.trim_end().ends_with(":_himitsu_secrets_fuzzy' \\"), "expected exec ref positional to use fuzzy helper, got:\n{ref_line}" diff --git a/rust/src/cli/doctor.rs b/rust/src/cli/doctor.rs index 5e8cae7..aae5405 100644 --- a/rust/src/cli/doctor.rs +++ b/rust/src/cli/doctor.rs @@ -292,6 +292,7 @@ mod tests { key_provider: KeyProvider::Disk, project_root: None, git: std::sync::Arc::new(crate::git::CliGitAdapter), + project_config_cell: Default::default(), } } diff --git a/rust/src/cli/exec.rs b/rust/src/cli/exec.rs index b46d697..3217597 100644 --- a/rust/src/cli/exec.rs +++ b/rust/src/cli/exec.rs @@ -30,14 +30,17 @@ use crate::remote::store; /// Run a command with secrets injected as environment variables. #[derive(Debug, Args)] pub struct ExecArgs { - /// Secret reference — selector grammar: + /// Secret references — one or more, each using the selector grammar: /// * `tag:NAME` — all secrets tagged NAME /// * `tag:A+tag:B` — AND-combined tags /// * `prod/*` — path glob /// * `prod/*+tag:pci` — glob AND tag /// * `prod/API_KEY` — concrete path - #[arg(value_name = "REF")] - pub r#ref: String, + /// + /// The union of all refs is injected. An env-var resolving to different + /// values via different refs is a hard error. + #[arg(value_name = "REF", required = true, num_args = 1..)] + pub refs: Vec, /// Filter resolved secrets by tag. Repeat for AND-semantics. Secrets /// missing any required tag are dropped before injection. @@ -49,9 +52,9 @@ pub struct ExecArgs { #[arg(long, short = 'i')] pub clean: bool, - /// Command and arguments to run. Pass after `--` so `himitsu` does not - /// try to interpret the command's own flags. - #[arg(trailing_var_arg = true, allow_hyphen_values = true, required = true)] + /// Command and arguments to run, after `--` (e.g. + /// `himitsu exec tag:pci prod/* -- node app.js`). + #[arg(last = true, required = true)] pub command: Vec, } @@ -62,8 +65,21 @@ pub fn run(args: ExecArgs, ctx: &Context) -> Result<()> { })?; } - if let Some(warning) = warn_if_shell_expanded(&args.r#ref) { - eprintln!("{warning}"); + for ref_str in &args.refs { + if let Some(warning) = warn_if_shell_expanded(ref_str) { + eprintln!("{warning}"); + } + // Reject documented-but-unsupported cross-store refs up front with a + // clear error, rather than silently parsing them as a local path and + // reporting "matched no secrets". A provider-qualified ref (e.g. + // `github:org/repo/prod/API_KEY`) has a colon-prefixed provider that + // is NOT the `tag:` selector prefix. + if is_cross_store_ref(ref_str) { + return Err(HimitsuError::NotSupported(format!( + "cross-store exec ref {ref_str:?} is not supported yet — run from the owning \ + store with `-r exec `", + ))); + } } let (cmd, cmd_args) = args @@ -71,31 +87,58 @@ pub fn run(args: ExecArgs, ctx: &Context) -> Result<()> { .split_first() .expect("clap enforces required = true on `command`"); - // Reject documented-but-unsupported cross-store refs up front with a clear - // error, rather than silently parsing them as a local path and reporting - // "matched no secrets". A provider-qualified ref (e.g. - // `github:org/repo/prod/API_KEY`) has a colon-prefixed provider that is NOT - // the `tag:` selector prefix. - if is_cross_store_ref(&args.r#ref) { - return Err(HimitsuError::NotSupported(format!( - "cross-store exec ref {:?} is not supported yet — run from the owning \ - store with `-r exec `", - args.r#ref - ))); - } - - // 1. Try to resolve the ref as an `outputs:` label from project config - // (e.g. `himitsu exec pci-prod`). Falls through to selector/glob/path - // parsing when the ref is not a defined output name. - if let Some(env_map) = try_resolve_output_label(ctx, &args.r#ref, &args.tags)? { - if env_map.is_empty() { - return Err(HimitsuError::ExecEmptyMatch(args.r#ref.clone())); + let outputs = super::output_resolver::OutputResolver::open(ctx)?; + + // Union of every ref's resolution. Each ref is tried as an `outputs:` + // label first (`env_map` is tri-state: `Ok(None)` means not a defined + // output name) and falls through to selector/glob/path parsing. Each ref + // must match at least one secret. The same env-var arriving from two refs + // with the same value is tolerated (overlapping refs hitting the same + // secret); different values are a hard conflict. + let mut env_map: BTreeMap = BTreeMap::new(); + let mut first_source: BTreeMap = BTreeMap::new(); + for ref_str in &args.refs { + let resolved = match outputs.env_map(ref_str, &args.tags)? { + Some(map) => map, + None => resolve_selector_env(ctx, ref_str, &args.tags)?, + }; + if resolved.is_empty() { + return Err(HimitsuError::ExecEmptyMatch(ref_str.clone())); + } + for (key, value) in resolved { + match env_map.get(&key) { + Some(prev) if *prev != value => { + let prev_ref = first_source + .get(&key) + .map(String::as_str) + .unwrap_or("an earlier ref"); + return Err(HimitsuError::InvalidConfig(format!( + "env-var {key:?} resolves to different values via {prev_ref:?} \ + and {ref_str:?}; narrow the refs or rename one via `set --env-key`", + ))); + } + _ => { + first_source + .entry(key.clone()) + .or_insert_with(|| ref_str.clone()); + env_map.insert(key, value); + } + } } - return spawn_and_wait(cmd, cmd_args, env_map, args.clean); } - // 2. Selector grammar: tag:/glob/concrete-path. - let selector = Selector::parse(&args.r#ref)?; + spawn_and_wait(cmd, cmd_args, env_map, args.clean) +} + +/// Resolve one selector-grammar ref (`tag:`/glob/concrete path) against the +/// active store to an env map. Errors with [`HimitsuError::ExecEmptyMatch`] +/// when nothing matches. +fn resolve_selector_env( + ctx: &Context, + ref_str: &str, + want_tags: &[String], +) -> Result> { + let selector = Selector::parse(ref_str)?; let all_paths = store::list_secrets(&ctx.store, None)?; let candidates: Vec = all_paths @@ -104,18 +147,12 @@ pub fn run(args: ExecArgs, ctx: &Context) -> Result<()> { .collect(); if candidates.is_empty() { - return Err(HimitsuError::ExecEmptyMatch(args.r#ref.clone())); + return Err(HimitsuError::ExecEmptyMatch(ref_str.to_string())); } let identities = ctx.load_identities()?; let decrypted = decrypt_paths(ctx, &identities, candidates)?; - let env_map = build_env_map(decrypted, &selector, &args.tags)?; - - if env_map.is_empty() { - return Err(HimitsuError::ExecEmptyMatch(args.r#ref.clone())); - } - - spawn_and_wait(cmd, cmd_args, env_map, args.clean) + build_env_map(decrypted, &selector, want_tags) } /// Returns `true` when `ref_str` is a provider-qualified cross-store ref @@ -140,122 +177,6 @@ fn is_cross_store_ref(ref_str: &str) -> bool { .all(|c| c.is_ascii_alphanumeric() || c == '-' || c == '_') } -/// Attempt to resolve `ref_str` as a named `outputs:` label from the project -/// config. Returns: -/// - `Ok(Some(env_map))` when the ref matches a defined output (the map may be -/// empty if the output's selectors matched no secrets); -/// - `Ok(None)` when the ref is NOT a defined output name (caller falls back to -/// selector parsing); -/// - `Err(..)` on a resolution/decryption failure. -/// -/// Unlike `codegen`, candidates are built WITH real tags (by decrypting each -/// secret) so `tag:` selectors inside an output block resolve correctly. -fn try_resolve_output_label( - ctx: &Context, - ref_str: &str, - want_tags: &[String], -) -> Result>> { - use crate::config::outputs::resolver::{ - resolve_outputs, Context as ResolverContext, SecretCandidate, - }; - - // Load the project config from the explicit project root when one was - // selected (`--project[=]`), so output labels resolve even when exec - // is invoked from a different cwd. Fall back to a cwd walk otherwise. - let project_config = match ctx.project_root.as_deref() { - Some(root) => config::load_project_config_from(root)?, - None => config::load_project_config()?, - }; - let outputs_map = project_config - .map(|(cfg, _)| cfg.outputs) - .unwrap_or_default(); - if outputs_map.is_empty() { - return Ok(None); - } - - // Build candidates with real tags so tag-selectors resolve. We decrypt - // every secret once; identities that can't decrypt a secret yield no tags - // for it (it simply won't match tag selectors), mirroring `ls --tag`. - let identities = ctx.load_identities()?; - let all_paths = store::list_secrets(&ctx.store, None)?; - let mut decoded_by_path: BTreeMap = BTreeMap::new(); - let mut candidates = Vec::with_capacity(all_paths.len()); - for path in all_paths { - let tags = match super::get::get_decoded_with_identities(ctx, &path, &identities) { - Ok(decoded) => { - let tags = decoded.tags.clone(); - decoded_by_path.insert(path.clone(), decoded); - tags - } - Err(_) => Vec::new(), - }; - candidates.push(SecretCandidate { path, tags }); - } - - let resolver_ctx = ResolverContext { - available_secrets: candidates, - }; - let resolved = resolve_outputs(&outputs_map, &resolver_ctx)?; - - let Some(output) = resolved.into_iter().find(|o| o.name == ref_str) else { - return Ok(None); - }; - - // Build the env map from resolved entries. Cross-store entries - // (`store_slug.is_some()`) are not supported by exec yet — surface a clear - // error rather than silently dropping them. - let mut env_map: BTreeMap = BTreeMap::new(); - for entry in output.entries { - if entry.store_slug.is_some() { - return Err(HimitsuError::NotSupported(format!( - "output {ref_str:?} references a cross-store secret ({}); \ - cross-store exec is not supported yet", - entry.secret_path - ))); - } - - // Decode the secret: reuse the already-decrypted value when available, - // otherwise decrypt now (an alias may point at a path not in the tag - // scan, though it usually is). - let decoded = match decoded_by_path.remove(&entry.secret_path) { - Some(d) => d, - None => super::get::get_decoded_with_identities(ctx, &entry.secret_path, &identities)?, - }; - - // Apply the `--tag` AND filter on top of the resolved selection. - if !want_tags.is_empty() - && !want_tags - .iter() - .all(|t| decoded.tags.iter().any(|d| d == t)) - { - continue; - } - - let key = entry.env_key; - super::set::validate_env_key(&key).map_err(|e| { - HimitsuError::InvalidReference(format!("{e} (from {:?})", entry.secret_path)) - })?; - if let Some((_, prev_path)) = env_map.get(&key) { - return Err(HimitsuError::InvalidConfig(format!( - "env-var {key:?} would be set by both {prev_path:?} and {:?}; \ - rename one via `set --env-key` or a selector alias", - entry.secret_path - ))); - } - let value = String::from_utf8(decoded.data).map_err(|e| { - HimitsuError::InvalidReference(format!( - "secret {:?} contains non-UTF-8 bytes — exec can only inject text values: {e}", - entry.secret_path - )) - })?; - env_map.insert(key, (value, entry.secret_path)); - } - - Ok(Some( - env_map.into_iter().map(|(k, (v, _))| (k, v)).collect(), - )) -} - /// Returns true if the path could match any group in the selector based on /// path/glob tokens alone. Tag tokens always pass (need decryption to check). fn is_path_candidate(selector: &Selector, path: &str) -> bool { @@ -582,7 +503,7 @@ mod tests { let cli = Cli::try_parse_from(["test", "prod/API_KEY", "--", "node", "-e", "console.log(1)"]) .unwrap(); - assert_eq!(cli.args.r#ref, "prod/API_KEY"); + assert_eq!(cli.args.refs, vec!["prod/API_KEY"]); assert_eq!(cli.args.command, vec!["node", "-e", "console.log(1)"]); assert!(!cli.args.clean); assert!(cli.args.tags.is_empty()); @@ -591,16 +512,37 @@ mod tests { "test", "prod/*", "--tag", "pci", "--tag", "rotate", "-i", "--", "env", ]) .unwrap(); - assert_eq!(cli.args.r#ref, "prod/*"); + assert_eq!(cli.args.refs, vec!["prod/*"]); assert_eq!(cli.args.tags, vec!["pci", "rotate"]); assert!(cli.args.clean); assert_eq!(cli.args.command, vec!["env"]); let cli = Cli::try_parse_from(["test", "tag:pci", "--", "env"]).unwrap(); - assert_eq!(cli.args.r#ref, "tag:pci"); + assert_eq!(cli.args.refs, vec!["tag:pci"]); let cli = Cli::try_parse_from(["test", "tag:pci+tag:prod", "--", "env"]).unwrap(); - assert_eq!(cli.args.r#ref, "tag:pci+tag:prod"); + assert_eq!(cli.args.refs, vec!["tag:pci+tag:prod"]); + } + + #[test] + fn parses_multiple_refs_before_double_dash() { + use clap::Parser; + + #[derive(Parser, Debug)] + struct Cli { + #[command(flatten)] + args: ExecArgs, + } + + let cli = Cli::try_parse_from(["test", "tag:pci", "prod/*", "app", "--", "env"]).unwrap(); + assert_eq!(cli.args.refs, vec!["tag:pci", "prod/*", "app"]); + assert_eq!(cli.args.command, vec!["env"]); + + // Without `--`, everything is a ref and the required command is + // missing — a clear parse error instead of silently treating the + // second token as the command. + let res = Cli::try_parse_from(["test", "foo", "bar"]); + assert!(res.is_err()); } #[test] diff --git a/rust/src/cli/export.rs b/rust/src/cli/export.rs index 5a1b14a..f70c1c2 100644 --- a/rust/src/cli/export.rs +++ b/rust/src/cli/export.rs @@ -6,7 +6,7 @@ use std::process::{Command as StdCommand, Stdio}; use clap::Args; use crate::cli::Context; -use crate::config::{load_project_config, ProjectConfig}; +use crate::config::ProjectConfig; use crate::crypto::{age as crypto, secret_value}; use crate::error::{HimitsuError, Result}; use crate::remote::store; @@ -92,7 +92,7 @@ pub fn run(args: ExportArgs, ctx: &Context) -> Result<()> { } // Encrypt via SOPS and write to file. - let project_cfg = load_project_config()?.map(|(c, _)| c); + let project_cfg = ctx.project_config()?.map(|(c, _)| c); let out_path = resolve_output_path(&args)?; write_encrypted(&out_path, &output_text, &args.format, project_cfg.as_ref())?; diff --git a/rust/src/cli/generate.rs b/rust/src/cli/generate.rs index a2e6906..04c9cae 100644 --- a/rust/src/cli/generate.rs +++ b/rust/src/cli/generate.rs @@ -6,11 +6,9 @@ use std::process::{Command as StdCommand, Stdio}; use clap::Args; use crate::cli::Context; -use crate::config::outputs::resolver::{resolve_outputs, Context as ResolverContext}; -use crate::config::{self, load_project_config, ProjectConfig}; -use crate::crypto::{age as crypto, secret_value}; +use crate::config::outputs::resolver::ResolvedOutput; +use crate::config::ProjectConfig; use crate::error::{HimitsuError, Result}; -use crate::remote::store; /// Generate SOPS-encrypted (or plaintext) output files from outputs definitions. #[derive(Debug, Args)] @@ -39,14 +37,14 @@ pub fn run(args: GenerateArgs, ctx: &Context) -> Result<()> { )); } - let project_cfg = load_project_config()?.map(|(cfg, _)| cfg); + let project_cfg = ctx.project_config()?.map(|(cfg, _)| cfg); - let outputs_map = project_cfg - .as_ref() - .map(|c| c.outputs.clone()) - .unwrap_or_default(); + // One resolution pass for the whole command: candidates carry real + // decrypted tags (so `tag:` selectors and selector-valued aliases + // resolve) and the same pass feeds the decoded values below. + let outputs = super::output_resolver::OutputResolver::open(ctx)?; - if outputs_map.is_empty() { + if outputs.all().is_empty() { return Err(HimitsuError::GenerateError( "no `outputs` defined in project config — \ define outputs: blocks in himitsu.yaml or use `himitsu codegen`" @@ -54,31 +52,19 @@ pub fn run(args: GenerateArgs, ctx: &Context) -> Result<()> { )); } - // Build candidates with real tags so `tag:` selectors and selector-valued - // aliases inside outputs: blocks resolve (empty tags made them match - // nothing). - let available_secrets = super::resolver_candidates_with_tags(ctx); - let resolver_ctx = ResolverContext { available_secrets }; - - let all_outputs = resolve_outputs(&outputs_map, &resolver_ctx)?; - - let to_generate: Vec<_> = if let Some(ref name) = args.output { - let filtered: Vec<_> = all_outputs - .into_iter() - .filter(|o| &o.name == name) - .collect(); - if filtered.is_empty() { - return Err(HimitsuError::GenerateError(format!( - "output '{name}' not found in project config" - ))); + let to_generate: Vec<&ResolvedOutput> = if let Some(ref name) = args.output { + match outputs.get(name) { + Some(o) => vec![o], + None => { + return Err(HimitsuError::GenerateError(format!( + "output '{name}' not found in project config" + ))) + } } - filtered } else { - all_outputs + outputs.all().iter().collect() }; - let identities = ctx.load_identities()?; - for resolved_output in &to_generate { if resolved_output.entries.is_empty() { eprintln!( @@ -89,37 +75,17 @@ pub fn run(args: GenerateArgs, ctx: &Context) -> Result<()> { } let mut output: BTreeMap = BTreeMap::new(); - for entry in &resolved_output.entries { - let effective_store = if let Some(ref slug) = entry.store_slug { - config::ensure_store(slug)? - } else { - ctx.store.clone() - }; - let payload = store::read_secret_payload(&effective_store, &entry.secret_path)?; - let plaintext = match crypto::decrypt_with_identities(&payload.ciphertext, &identities) - { - Ok(p) => p, - Err(_) if payload.legacy_proto_envelope => payload.ciphertext, - Err(err) => return Err(err), - }; - let decoded = secret_value::decode_with_legacy_environment( - &plaintext, - payload.legacy_environment.as_deref(), - ); - super::get::warn_if_expired(&entry.secret_path, &decoded); - let value = String::from_utf8(decoded.data).map_err(|e| { - HimitsuError::DecryptionFailed(format!( - "non-UTF-8 secret at '{}': {e}", - entry.secret_path - )) - })?; + for entry in outputs.decode(resolved_output)? { + if let Some(warning) = entry.expiry_warning() { + eprintln!("{warning}"); + } if output.contains_key(&entry.env_key) { eprintln!( "warning: duplicate key '{}' in output '{}' — using last value", entry.env_key, resolved_output.name ); } - output.insert(entry.env_key.clone(), value); + output.insert(entry.env_key, entry.value); } let yaml = build_plaintext_yaml(&output, &resolved_output.name); diff --git a/rust/src/cli/get.rs b/rust/src/cli/get.rs index 6045ef1..6d8b73a 100644 --- a/rust/src/cli/get.rs +++ b/rust/src/cli/get.rs @@ -144,18 +144,25 @@ fn emit_metadata_block(decoded: &secret_value::Decoded) { } pub(crate) fn warn_if_expired(path: &str, decoded: &secret_value::Decoded) { - let Some(ts) = decoded.expires_at.as_ref() else { - return; - }; - if duration::is_unset(ts) { - return; + if let Some(msg) = expiry_message(path, decoded.expires_at.as_ref()) { + eprintln!("{msg}"); } - let Some(dt) = duration::from_proto_timestamp(ts) else { - return; - }; - if dt <= chrono::Utc::now() { - eprintln!("warning: secret '{path}' expired at {}", dt.to_rfc3339()); +} + +/// Canonical expired-secret message, or `None` when no expiry is set, the +/// timestamp is unset, or it hasn't passed yet. Channel-free: callers decide +/// where it renders (CLI stderr, TUI badge). +pub(crate) fn expiry_message( + path: &str, + expires_at: Option<&pbjson_types::Timestamp>, +) -> Option { + let ts = expires_at?; + if duration::is_unset(ts) { + return None; } + let dt = duration::from_proto_timestamp(ts)?; + (dt <= chrono::Utc::now()) + .then(|| format!("warning: secret '{path}' expired at {}", dt.to_rfc3339())) } fn colorize(s: &str, sev: ExpirySeverity, is_tty: bool) -> String { diff --git a/rust/src/cli/import.rs b/rust/src/cli/import.rs index e4e01e5..41b2ef2 100644 --- a/rust/src/cli/import.rs +++ b/rust/src/cli/import.rs @@ -886,6 +886,7 @@ mod tests { key_provider: crate::config::KeyProvider::default(), project_root: None, git: std::sync::Arc::new(crate::git::CliGitAdapter), + project_config_cell: Default::default(), }; let err = run(args, &ctx).unwrap_err(); assert!( @@ -915,6 +916,7 @@ mod tests { key_provider: crate::config::KeyProvider::default(), project_root: None, git: std::sync::Arc::new(crate::git::CliGitAdapter), + project_config_cell: Default::default(), }; let err = run(args, &ctx).unwrap_err(); assert!( @@ -945,6 +947,7 @@ mod tests { key_provider: crate::config::KeyProvider::default(), project_root: None, git: std::sync::Arc::new(crate::git::CliGitAdapter), + project_config_cell: Default::default(), }; let err = run(args, &ctx).unwrap_err(); assert!( diff --git a/rust/src/cli/init.rs b/rust/src/cli/init.rs index 3ae36e3..e75323b 100644 --- a/rust/src/cli/init.rs +++ b/rust/src/cli/init.rs @@ -73,6 +73,7 @@ pub fn run(args: InitArgs, ctx: &Context) -> Result<()> { key_provider: ctx.key_provider.clone(), project_root: ctx.project_root.clone(), git: ctx.git.clone(), + project_config_cell: ctx.project_config_cell.clone(), }; return run_init(args, &patched_ctx); } diff --git a/rust/src/cli/join.rs b/rust/src/cli/join.rs index 0b7d590..c490fd9 100644 --- a/rust/src/cli/join.rs +++ b/rust/src/cli/join.rs @@ -21,17 +21,41 @@ pub struct JoinArgs { } pub fn run(args: JoinArgs, ctx: &Context) -> Result<()> { + // Presentation wrapper: the silent core does the work (the dispatcher + // runs the commit/push/cache chain post-dispatch); this layer owns the + // human-facing output. + match join_core(ctx, args.name)? { + JoinOutcome::AlreadyRecipient => { + println!("Already a recipient of this store — nothing to do."); + } + JoinOutcome::Joined(name) => { + println!("Joined as recipient '{name}'"); + } + } + Ok(()) +} + +/// Outcome of [`join_core`], for presentation layers to render. +#[derive(Debug)] +pub enum JoinOutcome { + /// The caller's key was added under this recipient name. + Joined(String), + /// The caller was already a recipient — nothing changed. + AlreadyRecipient, +} + +/// Silent core of `join`: add the caller's own age public key to the +/// store's recipient list. Idempotent. No stdin, no stdout — safe to call +/// from the TUI (use `store_ops::join`, which adds the mutation chain). +pub(super) fn join_core(ctx: &Context, name: Option) -> Result { let self_pubkey = read_own_pubkey(ctx)?; let recipients = age::collect_recipients(&ctx.store, ctx.recipients_path.as_deref())?; - let already_member = recipients.iter().any(|r| r.to_string() == self_pubkey); - - if already_member { - println!("Already a recipient of this store — nothing to do."); - return Ok(()); + if recipients.iter().any(|r| r.to_string() == self_pubkey) { + return Ok(JoinOutcome::AlreadyRecipient); } - let name = args.name.unwrap_or_else(default_recipient_name); + let name = name.unwrap_or_else(default_recipient_name); let recipients_dir = rstore::recipients_dir_with_override(&ctx.store, ctx.recipients_path.as_deref()); std::fs::create_dir_all(&recipients_dir)?; @@ -44,8 +68,7 @@ pub fn run(args: JoinArgs, ctx: &Context) -> Result<()> { if pub_file.exists() { let existing = std::fs::read_to_string(&pub_file)?.trim().to_string(); if existing == self_pubkey { - println!("Already a recipient of this store — nothing to do."); - return Ok(()); + return Ok(JoinOutcome::AlreadyRecipient); } return Err(HimitsuError::Recipient(format!( "recipient name '{name}' is taken by a different key — use --name to pick another" @@ -53,9 +76,7 @@ pub fn run(args: JoinArgs, ctx: &Context) -> Result<()> { } std::fs::write(&pub_file, format!("{self_pubkey}\n"))?; - println!("Joined as recipient '{name}'"); - - Ok(()) + Ok(JoinOutcome::Joined(name)) } fn read_own_pubkey(ctx: &Context) -> Result { @@ -122,6 +143,7 @@ mod tests { key_provider: crate::config::KeyProvider::default(), project_root: None, git: std::sync::Arc::new(crate::git::CliGitAdapter), + project_config_cell: Default::default(), } } diff --git a/rust/src/cli/mod.rs b/rust/src/cli/mod.rs index 565bdf7..fcf0d56 100644 --- a/rust/src/cli/mod.rs +++ b/rust/src/cli/mod.rs @@ -20,6 +20,7 @@ pub mod join; pub mod keys; pub mod ls; pub mod migrate; +pub mod output_resolver; pub mod prime; pub mod read; pub mod recipient; @@ -30,6 +31,7 @@ pub mod schema; pub mod search; pub mod set; pub mod share; +pub mod store_ops; pub mod sync; pub mod tag; pub mod write; @@ -43,6 +45,12 @@ use tracing::debug; use crate::error::{HimitsuError, Result}; +/// Shared memo cell for the lazily-loaded project config. `Default` yields +/// an empty cell; `Context` clones share one cell, so the config loads (and +/// the legacy `envs:` migration warning fires) at most once per process. +pub type ProjectConfigCell = + Arc>>; + /// Resolved paths for the current invocation. #[derive(Clone)] pub struct Context { @@ -68,6 +76,35 @@ pub struct Context { /// Git operations backend. Production uses `CliGitAdapter` (shells out to /// `git`); tests can substitute `InMemoryGitAdapter`. pub git: Arc, + /// Memoized project config for this invocation. Populated lazily by + /// [`Self::project_config`] (or seeded at construction when the + /// dispatcher already loaded it for the recipients override). + pub project_config_cell: ProjectConfigCell, +} + +// ── Project config ─────────────────────────────────────────────── +impl Context { + /// The project config that applies to this invocation. + /// + /// Owns the "which project config applies" decision: an explicit + /// `--project[=]` root wins over a cwd walk. Loaded lazily on + /// first call and memoized (clones share the memo), so commands and + /// views can consult the config freely without re-reading it or + /// re-triggering the legacy `envs:` warning. Load errors (e.g. + /// malformed YAML) are returned but not memoized. + pub fn project_config( + &self, + ) -> Result> { + if let Some(cached) = self.project_config_cell.get() { + return Ok(cached.clone()); + } + let loaded = match self.project_root.as_deref() { + Some(root) => crate::config::load_project_config_from(root)?, + None => crate::config::load_project_config()?, + }; + let _ = self.project_config_cell.set(loaded.clone()); + Ok(loaded) + } } // ── Path resolution ────────────────────────────────────────────── @@ -553,6 +590,7 @@ impl Cli { key_provider: crate::config::KeyProvider::default(), project_root: None, git: Arc::new(crate::git::CliGitAdapter), + project_config_cell: Default::default(), }; init::run( init::InitArgs { @@ -616,7 +654,12 @@ impl Cli { init::ensure_default_origin(&store, &state_dir.join("stores")); } - let recipients_path = load_recipients_path_override(&store, &selector); + // One cell for the whole invocation: the recipients override may load + // the project config during construction (project mode); seeding the + // cell there means `ctx.project_config()` never re-reads it. + let project_config_cell = ProjectConfigCell::default(); + let recipients_path = + load_recipients_path_override(&store, &selector, &project_config_cell); let key_provider = crate::config::Config::load(&crate::config::config_path()) .map(|c| c.key_provider) .unwrap_or_default(); @@ -632,6 +675,7 @@ impl Cli { key_provider, project_root, git: Arc::new(crate::git::CliGitAdapter), + project_config_cell, }; // Pre-dispatch: when `auto_pull` is on, fetch + fast-forward the @@ -703,25 +747,13 @@ impl Cli { Command::Doctor(args) => doctor::run(args, &ctx), }; - // Post-dispatch: enforce the append-only invariant for mutating - // commands. Always commit (success OR failure) so `git status` is - // never left dirty; on failure prefix the message with `FAILED:` and - // append the error so the history records the partial state. Push - // only on success, and only when the user did not opt out. + // Post-dispatch: run the append-only mutation chain (commit on + // success or failure, push, completions-cache refresh) — owned by + // `store_ops` so the CLI dispatcher and the TUI's mutation cores + // can't drift. One finalize per command keeps batch commands + // (e.g. `import`) a single commit. if let Some(msg) = mutation_msg { - let final_msg = match &result { - Ok(_) => format!("himitsu: {msg}"), - Err(e) => format!("himitsu: FAILED: {msg}: {e}"), - }; - let committed = ctx.commit(&final_msg); - if result.is_ok() && committed && !no_push { - ctx.push(); - } - // Keep the completions cache in sync after every successful mutation - // so the next tab-press sees the updated path list immediately. - if result.is_ok() && !ctx.store.as_os_str().is_empty() { - let _ = crate::completions_cache::refresh_store(&ctx.state_dir, &ctx.store); - } + store_ops::finalize(&ctx, &msg, no_push, &result); } result @@ -742,7 +774,9 @@ impl Cli { // than erroring out. Bare-TUI launch uses Global context — explicit // project mode is reserved for the parent epic's context chooser. let store = crate::config::resolve_store(None).unwrap_or_default(); - let recipients_path = load_recipients_path_override(&store, &ContextSelector::Global); + let project_config_cell = ProjectConfigCell::default(); + let recipients_path = + load_recipients_path_override(&store, &ContextSelector::Global, &project_config_cell); let key_provider = crate::config::Config::load(&crate::config::config_path()) .map(|c| c.key_provider) .unwrap_or_default(); @@ -755,6 +789,7 @@ impl Cli { key_provider, project_root: None, git: Arc::new(crate::git::CliGitAdapter), + project_config_cell, }; crate::tui::run(&ctx) } @@ -876,6 +911,7 @@ fn prompt_to_create_store(store: &Path, data_dir: &Path, state_dir: &Path) -> Re fn load_recipients_path_override( store: &std::path::Path, selector: &ContextSelector, + project_config_cell: &ProjectConfigCell, ) -> Option { if store.as_os_str().is_empty() { return None; @@ -888,9 +924,14 @@ fn load_recipients_path_override( } if let ContextSelector::Project(root) = selector { - if let Ok(Some((project_cfg, _))) = crate::config::load_project_config_from(root) { - if project_cfg.recipients_path.is_some() { - return project_cfg.recipients_path; + if let Ok(loaded) = crate::config::load_project_config_from(root) { + // Seed the invocation's memo so `Context::project_config()` + // doesn't re-read (or re-warn about) the same file. + let _ = project_config_cell.set(loaded.clone()); + if let Some((project_cfg, _)) = loaded { + if project_cfg.recipients_path.is_some() { + return project_cfg.recipients_path; + } } } } @@ -898,20 +939,6 @@ fn load_recipients_path_override( None } -/// Build resolver candidates for the store with REAL tags, by decrypting each -/// secret. Tag selectors and selector-valued aliases inside `outputs:` blocks -/// only resolve when candidates carry their tags, so `exec`, `generate`, and -/// `codegen` share this builder instead of passing `tags: vec![]` (which made -/// every `tag:` selector silently match nothing). -/// -/// Secrets the current identity cannot decrypt contribute no tags (they simply -/// won't match tag selectors), mirroring `ls --tag`. -pub(crate) fn resolver_candidates_with_tags( - ctx: &Context, -) -> Vec { - resolver::SecretResolver::resolve_candidates(ctx) -} - #[cfg(test)] mod tests { use super::*; @@ -1061,9 +1088,42 @@ mod tests { key_provider: crate::config::KeyProvider::default(), project_root: None, git: Arc::new(crate::git::CliGitAdapter), + project_config_cell: Default::default(), } } + #[test] + fn project_config_prefers_explicit_root_and_memoizes() { + let project = tempfile::tempdir().unwrap(); + std::fs::write( + project.path().join("himitsu.yaml"), + "default_store: \"acme/secrets\"\n", + ) + .unwrap(); + + let mut ctx = ctx_for(Path::new("")); + ctx.project_root = Some(project.path().to_path_buf()); + + // The explicit project root wins over any cwd walk. + let (cfg, path) = ctx + .project_config() + .unwrap() + .expect("config under explicit root"); + assert_eq!(cfg.default_store.as_deref(), Some("acme/secrets")); + assert!(path.starts_with(project.path())); + + // Memoized and shared across clones: rewriting the file on disk is + // not observed by a clone of the same invocation. + let clone = ctx.clone(); + std::fs::write( + project.path().join("himitsu.yaml"), + "default_store: \"other/store\"\n", + ) + .unwrap(); + let (cfg2, _) = clone.project_config().unwrap().expect("memoized config"); + assert_eq!(cfg2.default_store.as_deref(), Some("acme/secrets")); + } + #[test] fn pull_if_remote_fast_forwards_when_remote_advances() { let root = tempfile::tempdir().unwrap(); diff --git a/rust/src/cli/output_resolver.rs b/rust/src/cli/output_resolver.rs new file mode 100644 index 0000000..47db9eb --- /dev/null +++ b/rust/src/cli/output_resolver.rs @@ -0,0 +1,570 @@ +//! OutputResolver — the deepened Output pipeline. +//! +//! One module owns the path from the project config `outputs:` map to +//! decoded entries: candidates with decrypted tags → selector/alias +//! resolution → values. Consumed by `exec`, `generate`, and `codegen` +//! (and, later, the TUI outputs view), replacing the pipeline those +//! commands previously assembled by hand — including the +//! `tags: vec![]` bug class where tag selectors silently matched +//! nothing, and the double decryption where candidates were decrypted +//! for tags and entries decrypted again for values. +//! +//! Invariants (see CONTEXT.md): +//! - **Cheap empty**: no project config or no `outputs:` map means no +//! identity load, no store I/O, no decryption. +//! - **Decrypt once**: `open` performs the single scan; tags for +//! selector matching and plaintexts for later materialization come +//! from the same pass. Secrets the current Identity cannot decrypt +//! contribute no tags (mirroring `ls --tag`) and only error when a +//! value is actually needed. +//! - **Whole-map validation**: selector parse errors and alias +//! exactly-one violations surface at `open`, even when the caller +//! eventually asks for a different Output. +//! - **Tri-state lookup**: [`OutputResolver::env_map`] returns +//! `Ok(None)` iff the label is not a defined Output, so `exec` can +//! fall through to selector parsing; real failures are `Err`. +//! - **Channel-free materialization**: [`DecodedEntry`] carries expiry +//! data; callers decide where warnings render (CLI stderr, TUI badge). + +use std::collections::BTreeMap; + +use super::Context; +use crate::config::outputs::dsl::OutputsMap; +use crate::config::outputs::resolver::{ + resolve_outputs, Context as ResolverContext, ResolvedOutput, SecretCandidate, +}; +use crate::crypto::secret_value; +use crate::error::{HimitsuError, Result}; +use crate::remote::store; + +/// One decoded env-var binding from an Output. +#[derive(Debug, Clone)] +pub struct DecodedEntry { + /// The env-var name (alias key, or derived from the secret path). + pub env_key: String, + /// The secret path within its store. + pub secret_path: String, + /// `Some` = cross-store entry (store slug); `None` = the active store. + /// Not yet read by the CLI callers — the TUI outputs view renders it. + #[allow(dead_code)] + pub store_slug: Option, + /// UTF-8-validated plaintext value. + pub value: String, + /// Expiration timestamp, when set on the Secret. + pub expires_at: Option, +} + +impl DecodedEntry { + /// Canonical expired-secret warning text, when this entry is expired. + /// Materialization is channel-free: CLI callers print this to stderr, + /// the TUI renders it as a badge. + pub fn expiry_warning(&self) -> Option { + super::get::expiry_message(&self.secret_path, self.expires_at.as_ref()) + } +} + +/// The project's `outputs:` map, resolved against the active store. +/// +/// [`OutputResolver::open`] is the one expensive call; everything after it +/// reads the snapshot. Borrowing [`Context`] encodes the invariant that a +/// resolver opened against one store is never materialized against another. +pub struct OutputResolver<'ctx> { + ctx: &'ctx Context, + /// Post-brace-expansion resolved Outputs, declaration order. + resolved: Vec, + /// Candidates from the `open` scan (path + decrypted tags), reused by + /// [`Self::resolve_map`] so previews don't re-decrypt the store. + #[allow(dead_code)] + candidates: Vec, + /// Plaintext-decoded local Secrets from the single `open` scan. + decoded: BTreeMap, + identities: Vec<::age::x25519::Identity>, +} + +impl<'ctx> OutputResolver<'ctx> { + /// Resolve the invocation's `outputs:` map (via + /// [`Context::project_config`]) against the active store. + /// + /// Cheap when no Outputs are defined: no identity load, no store I/O, + /// no decryption. Otherwise this is the single decrypt-once scan. + pub fn open(ctx: &'ctx Context) -> Result { + let outputs_map = ctx + .project_config()? + .map(|(cfg, _)| cfg.outputs) + .unwrap_or_default(); + if outputs_map.is_empty() { + return Ok(Self { + ctx, + resolved: Vec::new(), + candidates: Vec::new(), + decoded: BTreeMap::new(), + identities: Vec::new(), + }); + } + + let identities = ctx.load_identities()?; + let all_paths = store::list_secrets(&ctx.store, None)?; + let mut decoded: BTreeMap = BTreeMap::new(); + let mut candidates = Vec::with_capacity(all_paths.len()); + for path in all_paths { + let tags = match super::resolver::SecretResolver::resolve_with_identities( + ctx, &path, &identities, + ) { + Ok(d) => { + let tags = d.tags.clone(); + decoded.insert(path.clone(), d); + tags + } + Err(_) => Vec::new(), + }; + candidates.push(SecretCandidate { path, tags }); + } + + let resolver_ctx = ResolverContext { + available_secrets: candidates.clone(), + }; + let resolved = resolve_outputs(&outputs_map, &resolver_ctx)?; + Ok(Self { + ctx, + resolved, + candidates, + decoded, + identities, + }) + } + + /// All resolved Outputs (post brace-expansion), declaration order. + /// Metadata only — no plaintext values. + pub fn all(&self) -> &[ResolvedOutput] { + &self.resolved + } + + /// One resolved Output by exact (post-expansion) name. + pub fn get(&self, name: &str) -> Option<&ResolvedOutput> { + self.resolved.iter().find(|o| o.name == name) + } + + /// The strict `exec` surface: one label → injection-ready env map. + /// + /// Tri-state: `Ok(None)` iff `label` is not a defined Output (the + /// caller falls through to selector parsing); `Ok(Some(map))` when + /// defined — the map may be empty (emptiness policy is the caller's); + /// everything else is `Err`. + /// + /// Local-store only: any cross-store entry is a hard + /// [`HimitsuError::NotSupported`]. `tag_filter` is AND-applied to each + /// entry's decrypted tags *before* env-key conflict detection, so a + /// filtered-out entry can never cause a conflict. + pub fn env_map( + &self, + label: &str, + tag_filter: &[String], + ) -> Result>> { + let Some(output) = self.get(label) else { + return Ok(None); + }; + + let mut env_map: BTreeMap = BTreeMap::new(); + for entry in &output.entries { + if entry.store_slug.is_some() { + return Err(HimitsuError::NotSupported(format!( + "output {label:?} references a cross-store secret ({}); \ + cross-store exec is not supported yet", + entry.secret_path + ))); + } + + let decoded = self.decoded_for(entry)?; + + // Apply the `--tag` AND filter on top of the resolved selection. + if !tag_filter.is_empty() + && !tag_filter + .iter() + .all(|t| decoded.tags.iter().any(|d| d == t)) + { + continue; + } + + let key = entry.env_key.clone(); + super::set::validate_env_key(&key).map_err(|e| { + HimitsuError::InvalidReference(format!("{e} (from {:?})", entry.secret_path)) + })?; + if let Some((_, prev_path)) = env_map.get(&key) { + return Err(HimitsuError::InvalidConfig(format!( + "env-var {key:?} would be set by both {prev_path:?} and {:?}; \ + rename one via `set --env-key` or a selector alias", + entry.secret_path + ))); + } + let value = String::from_utf8(decoded.data).map_err(|e| { + HimitsuError::InvalidReference(format!( + "secret {:?} contains non-UTF-8 bytes — exec can only inject text values: {e}", + entry.secret_path + )) + })?; + env_map.insert(key, (value, entry.secret_path.clone())); + } + + Ok(Some( + env_map.into_iter().map(|(k, (v, _))| (k, v)).collect(), + )) + } + + /// Decode an Output's entries to plaintext values. + /// + /// Follows cross-store entries (via [`crate::config::ensure_store`]). + /// Preserves entry order and duplicate env keys — callers own their + /// collapse policy (generate warns then last-wins; codegen sops mode + /// silently last-wins; exec's strict collapse is [`Self::env_map`]). + pub fn decode(&self, output: &ResolvedOutput) -> Result> { + let mut entries = Vec::with_capacity(output.entries.len()); + for entry in &output.entries { + let decoded = if let Some(ref slug) = entry.store_slug { + let effective_store = crate::config::ensure_store(slug)?; + let payload = + store::read_secret_payload(&effective_store, &entry.secret_path)?; + match crate::crypto::age::decrypt_with_identities( + &payload.ciphertext, + &self.identities, + ) { + Ok(p) => secret_value::decode_with_legacy_environment( + &p, + payload.legacy_environment.as_deref(), + ), + Err(_) if payload.legacy_proto_envelope => { + secret_value::decode_with_legacy_environment( + &payload.ciphertext, + payload.legacy_environment.as_deref(), + ) + } + Err(err) => return Err(err), + } + } else { + self.decoded_for(entry)? + }; + + let value = String::from_utf8(decoded.data).map_err(|e| { + HimitsuError::DecryptionFailed(format!( + "non-UTF-8 secret at '{}': {e}", + entry.secret_path + )) + })?; + entries.push(DecodedEntry { + env_key: entry.env_key.clone(), + secret_path: entry.secret_path.clone(), + store_slug: entry.store_slug.clone(), + value, + expires_at: decoded.expires_at, + }); + } + Ok(entries) + } + + /// Resolve an arbitrary `outputs:` map against the candidates built at + /// [`Self::open`] — the seam for previewing an edited-but-unsaved map + /// (TUI DSL editor) and for tests, without re-decrypting the store. + #[allow(dead_code)] + pub fn resolve_map(&self, outputs: &OutputsMap) -> Result> { + let resolver_ctx = ResolverContext { + available_secrets: self.candidates.clone(), + }; + resolve_outputs(outputs, &resolver_ctx) + } + + /// A local entry's decoded value: from the `open` scan's cache when + /// available (the common case), otherwise decrypted now with the rich + /// [`SecretResolver`](super::resolver::SecretResolver) diagnostic — this + /// is where an undecryptable Secret that selectors tolerated becomes a + /// hard error because its *value* is needed. + fn decoded_for( + &self, + entry: &crate::config::outputs::resolver::ResolvedEntry, + ) -> Result { + match self.decoded.get(&entry.secret_path) { + Some(d) => Ok(d.clone()), + None => super::resolver::SecretResolver::resolve_with_identities( + self.ctx, + &entry.secret_path, + &self.identities, + ), + } + } +} + +#[cfg(test)] +mod tests { + use std::path::Path; + use std::sync::Arc; + + use secrecy::ExposeSecret; + + use super::*; + use crate::crypto::age as hage; + use crate::proto::SecretValue; + + /// A Context over a tempdir store with one real age identity on disk, + /// recipients configured, and `project_root` pointing at an (initially + /// config-less) project directory. + fn test_ctx() -> (tempfile::TempDir, Context) { + let tmp = tempfile::tempdir().unwrap(); + let data_dir = tmp.path().join("data"); + let state_dir = tmp.path().join("state"); + let store = tmp.path().join("store"); + let project = tmp.path().join("project"); + std::fs::create_dir_all(&data_dir).unwrap(); + std::fs::create_dir_all(store.join(".himitsu/recipients")).unwrap(); + std::fs::create_dir_all(store.join(".himitsu/secrets")).unwrap(); + std::fs::create_dir_all(&project).unwrap(); + + let identity = ::age::x25519::Identity::generate(); + let pubkey = identity.to_public().to_string(); + std::fs::write(data_dir.join("key"), identity.to_string().expose_secret()).unwrap(); + std::fs::write( + store.join(".himitsu/recipients/me.pub"), + format!("{pubkey}\n"), + ) + .unwrap(); + + let ctx = Context { + data_dir, + state_dir, + store, + recipients_path: None, + key_provider: crate::config::KeyProvider::default(), + project_root: Some(project), + git: Arc::new(crate::git::CliGitAdapter), + project_config_cell: Default::default(), + }; + (tmp, ctx) + } + + fn write_project_outputs(ctx: &Context, yaml: &str) { + let root = ctx.project_root.as_ref().unwrap(); + std::fs::write(root.join("himitsu.yaml"), yaml).unwrap(); + } + + fn write_secret_with( + store_path: &Path, + path: &str, + data: &[u8], + tags: &[&str], + recipients: &[::age::x25519::Recipient], + ) { + let sv = SecretValue { + data: data.to_vec(), + tags: tags.iter().map(|s| s.to_string()).collect(), + ..Default::default() + }; + let wire = secret_value::encode(&sv); + let ct = hage::encrypt(&wire, recipients).unwrap(); + store::write_secret(store_path, path, &ct).unwrap(); + } + + fn store_recipients(ctx: &Context) -> Vec<::age::x25519::Recipient> { + hage::collect_recipients(&ctx.store, None).unwrap() + } + + #[test] + fn open_is_cheap_when_no_outputs_are_defined() { + let (_tmp, mut ctx) = test_ctx(); + // No project config at all, no identity on disk, and a store path + // that doesn't exist: `open` must still succeed because the empty + // outputs map short-circuits before any identity load or store I/O. + ctx.data_dir = ctx.data_dir.join("nonexistent"); + ctx.store = ctx.store.join("nonexistent"); + + let outputs = OutputResolver::open(&ctx).expect("cheap empty open"); + assert!(outputs.all().is_empty()); + assert_eq!(outputs.env_map("anything", &[]).unwrap(), None); + assert!(outputs.get("anything").is_none()); + } + + #[test] + fn env_map_is_tri_state() { + let (_tmp, ctx) = test_ctx(); + let recipients = store_recipients(&ctx); + write_secret_with(&ctx.store, "prod/api-key", b"v1", &["pci"], &recipients); + write_project_outputs( + &ctx, + "outputs:\n app:\n selectors:\n - tag:pci\n", + ); + + let outputs = OutputResolver::open(&ctx).unwrap(); + // Unknown label: not an error — the caller falls through to + // selector parsing. + assert_eq!(outputs.env_map("nope", &[]).unwrap(), None); + // Defined label: resolved with real decrypted tags, value from the + // same single scan. + let env = outputs.env_map("app", &[]).unwrap().expect("defined"); + assert_eq!(env.get("API_KEY").map(String::as_str), Some("v1")); + } + + #[test] + fn undecryptable_secret_matches_no_tags_but_value_is_hard_error() { + let (_tmp, ctx) = test_ctx(); + let recipients = store_recipients(&ctx); + write_secret_with(&ctx.store, "prod/mine", b"ok", &["pci"], &recipients); + // A secret encrypted to a foreign identity: contributes no tags + // (tolerated), but erroring when its value is needed. + let foreign = ::age::x25519::Identity::generate(); + write_secret_with( + &ctx.store, + "prod/foreign", + b"locked", + &["pci"], + &[foreign.to_public()], + ); + write_project_outputs( + &ctx, + "outputs:\n tagged:\n selectors:\n - tag:pci\n direct:\n selectors: []\n aliases:\n LOCKED: prod/foreign\n", + ); + + let outputs = OutputResolver::open(&ctx).unwrap(); + // The foreign secret's tags are unreadable, so tag:pci matches only + // the decryptable one. + let env = outputs.env_map("tagged", &[]).unwrap().expect("defined"); + assert_eq!(env.len(), 1); + assert!(env.contains_key("MINE")); + // Naming it directly needs the value: hard error. + let err = outputs.env_map("direct", &[]).unwrap_err(); + assert!(matches!(err, HimitsuError::DecryptionFailed(_)), "{err:?}"); + } + + #[test] + fn cross_store_entry_is_rejected_by_env_map_before_any_value_work() { + let (_tmp, ctx) = test_ctx(); + let recipients = store_recipients(&ctx); + write_secret_with(&ctx.store, "prod/local", b"v", &[], &recipients); + write_project_outputs( + &ctx, + "outputs:\n app:\n selectors: []\n aliases:\n SHARED: github:acme/other#prod/key\n", + ); + + let outputs = OutputResolver::open(&ctx).unwrap(); + let err = outputs.env_map("app", &[]).unwrap_err(); + match err { + HimitsuError::NotSupported(msg) => { + assert!(msg.contains("cross-store"), "{msg}"); + assert!(msg.contains("prod/key"), "{msg}"); + } + other => panic!("expected NotSupported, got {other:?}"), + } + // The metadata surface still shows the entry, slug intact, for + // observers (decode/preview callers own their policy). + let entry = &outputs.get("app").unwrap().entries[0]; + assert_eq!(entry.store_slug.as_deref(), Some("acme/other")); + } + + #[test] + fn tag_filter_applies_before_conflict_detection() { + let (_tmp, ctx) = test_ctx(); + let recipients = store_recipients(&ctx); + // Both paths derive the same env key (API_KEY). + write_secret_with(&ctx.store, "prod/api-key", b"p", &["prod"], &recipients); + write_secret_with(&ctx.store, "staging/api-key", b"s", &[], &recipients); + write_project_outputs( + &ctx, + "outputs:\n app:\n selectors:\n - prod/*\n - staging/*\n", + ); + + let outputs = OutputResolver::open(&ctx).unwrap(); + // Unfiltered: two survivors bind API_KEY — hard conflict naming both. + let err = outputs.env_map("app", &[]).unwrap_err(); + match err { + HimitsuError::InvalidConfig(msg) => { + assert!(msg.contains("prod/api-key"), "{msg}"); + assert!(msg.contains("staging/api-key"), "{msg}"); + } + other => panic!("expected InvalidConfig, got {other:?}"), + } + // Filtered: the untagged collider is dropped before the conflict + // check, so this succeeds with one binding. + let env = outputs + .env_map("app", &["prod".to_string()]) + .unwrap() + .expect("defined"); + assert_eq!(env.get("API_KEY").map(String::as_str), Some("p")); + } + + #[test] + fn alias_exactly_one_violation_surfaces_at_open() { + let (_tmp, ctx) = test_ctx(); + let recipients = store_recipients(&ctx); + write_secret_with(&ctx.store, "prod/x", b"v", &[], &recipients); + // Selector-valued alias matching zero secrets: whole-map validation + // fails at open, even though a caller may want a different output. + write_project_outputs( + &ctx, + "outputs:\n ok:\n selectors:\n - prod/x\n broken:\n selectors: []\n aliases:\n K: tag:nomatch\n", + ); + + assert!(OutputResolver::open(&ctx).is_err()); + } + + #[test] + fn decode_preserves_duplicates_order_and_metadata() { + let (_tmp, ctx) = test_ctx(); + let recipients = store_recipients(&ctx); + write_secret_with(&ctx.store, "prod/api-key", b"p", &[], &recipients); + write_secret_with(&ctx.store, "staging/api-key", b"s", &[], &recipients); + write_project_outputs( + &ctx, + "outputs:\n app:\n selectors:\n - prod/*\n - staging/*\n", + ); + + let outputs = OutputResolver::open(&ctx).unwrap(); + let resolved = outputs.get("app").unwrap(); + let entries = outputs.decode(resolved).unwrap(); + // Duplicate env keys preserved in entry order — collapse policy + // belongs to the caller (generate warns, codegen last-wins). + assert_eq!(entries.len(), 2); + assert!(entries.iter().all(|e| e.env_key == "API_KEY")); + assert!(entries.iter().all(|e| e.store_slug.is_none())); + assert!(entries.iter().all(|e| e.expiry_warning().is_none())); + let values: Vec<&str> = entries.iter().map(|e| e.value.as_str()).collect(); + assert_eq!(values, ["p", "s"]); + } + + #[test] + fn non_utf8_value_is_a_hard_error() { + let (_tmp, ctx) = test_ctx(); + let recipients = store_recipients(&ctx); + write_secret_with(&ctx.store, "prod/blob", &[0xff, 0xfe, 0x00], &[], &recipients); + write_project_outputs( + &ctx, + "outputs:\n app:\n selectors:\n - prod/blob\n", + ); + + let outputs = OutputResolver::open(&ctx).unwrap(); + assert!(matches!( + outputs.env_map("app", &[]).unwrap_err(), + HimitsuError::InvalidReference(_) + )); + let resolved = outputs.get("app").unwrap(); + assert!(matches!( + outputs.decode(resolved).unwrap_err(), + HimitsuError::DecryptionFailed(_) + )); + } + + #[test] + fn resolve_map_previews_an_unsaved_map_against_the_open_scan() { + let (_tmp, ctx) = test_ctx(); + let recipients = store_recipients(&ctx); + write_secret_with(&ctx.store, "prod/api-key", b"v", &["pci"], &recipients); + write_project_outputs( + &ctx, + "outputs:\n app:\n selectors:\n - prod/api-key\n", + ); + + let outputs = OutputResolver::open(&ctx).unwrap(); + // Preview a map that is NOT the saved project config (e.g. the TUI + // DSL editor's buffer) without re-decrypting the store. + let preview_yaml = "preview:\n selectors:\n - tag:pci\n"; + let preview_map: OutputsMap = serde_yaml::from_str(preview_yaml).unwrap(); + let resolved = outputs.resolve_map(&preview_map).unwrap(); + assert_eq!(resolved.len(), 1); + assert_eq!(resolved[0].name, "preview"); + assert_eq!(resolved[0].entries[0].secret_path, "prod/api-key"); + } +} diff --git a/rust/src/cli/recipient.rs b/rust/src/cli/recipient.rs index 9596017..5ff9138 100644 --- a/rust/src/cli/recipient.rs +++ b/rust/src/cli/recipient.rs @@ -104,35 +104,6 @@ pub struct RecipientEntry { pub short_key: String, } -/// Add a recipient by explicit age public key (the `--age-key` path). Mirrors -/// `himitsu recipient add --age-key `, but never prompts and never -/// prints — safe to call from the TUI where stdin/stdout drive ratatui. -pub fn add_recipient( - ctx: &Context, - name: &str, - age_key: &str, - description: Option, -) -> Result<()> { - // Normalise an empty description to `None` so the silent core never - // writes an empty sidecar. The TUI must NOT reach the interactive prompt - // path in `add`, so we call the core directly. - let description = description.and_then(|d| { - let t = d.trim().to_string(); - if t.is_empty() { - None - } else { - Some(t) - } - }); - add_core(ctx, name, Some(age_key), description).map(|_missing| ()) -} - -/// Remove a recipient by name. Mirrors `himitsu recipient rm `, but -/// never prints — safe to call from the TUI. -pub fn remove_recipient(ctx: &Context, name: &str) -> Result<()> { - rm_core(ctx, name) -} - /// List recipients in the current store, sorted by name. Live read — no /// caching — so the TUI always reflects the on-disk state. pub fn list_recipients(ctx: &Context) -> Result> { @@ -233,7 +204,7 @@ fn add( /// /// Does NOT prompt and does NOT print — callers own presentation. The CLI's /// `add` adds prompting + stdout messages; the TUI calls this directly. -fn add_core( +pub(super) fn add_core( ctx: &Context, name: &str, age_key: Option<&str>, @@ -317,7 +288,7 @@ fn rm(ctx: &Context, name: &str) -> Result<()> { /// Silent mutation core for removing a recipient. Deletes the `.pub` and any /// sidecar. Does NOT print — callers own presentation. -fn rm_core(ctx: &Context, name: &str) -> Result<()> { +pub(super) fn rm_core(ctx: &Context, name: &str) -> Result<()> { let recipients_dir = flat_recipients_dir(ctx); let pub_file = recipients_dir.join(format!("{name}.pub")); @@ -618,6 +589,7 @@ mod tests { key_provider: crate::config::KeyProvider::default(), project_root: None, git: std::sync::Arc::new(crate::git::CliGitAdapter), + project_config_cell: Default::default(), }; (tmp, ctx) } @@ -768,11 +740,11 @@ mod tests { #[test] fn add_recipient_wrapper_with_empty_description_writes_no_sidecar() { - // Regression for the TUI hang: add_recipient (the TUI-facing wrapper) + // Regression for the TUI hang: store_ops::recipient_add (the TUI-facing core) // must NOT reach the interactive prompt path in `add`. An empty/None // description must be normalised to no sidecar via the silent core. let (_tmp, ctx) = mk_ctx(); - add_recipient(&ctx, "alice", AGE_KEY_1, None).unwrap(); + crate::cli::store_ops::recipient_add(&ctx, "alice", AGE_KEY_1, None).unwrap(); let rdir = rstore::recipients_dir(&ctx.store); assert!(rdir.join("alice.pub").exists()); assert!( @@ -781,7 +753,7 @@ mod tests { ); // A whitespace-only description is also treated as empty. - add_recipient(&ctx, "bob", AGE_KEY_2, Some(" ".into())).unwrap(); + crate::cli::store_ops::recipient_add(&ctx, "bob", AGE_KEY_2, Some(" ".into())).unwrap(); assert!(rdir.join("bob.pub").exists()); assert!(!rdir.join("bob.yaml").exists()); } @@ -789,7 +761,7 @@ mod tests { #[test] fn add_recipient_wrapper_with_description_writes_sidecar() { let (_tmp, ctx) = mk_ctx(); - add_recipient(&ctx, "alice", AGE_KEY_1, Some("Platform lead".into())).unwrap(); + crate::cli::store_ops::recipient_add(&ctx, "alice", AGE_KEY_1, Some("Platform lead".into())).unwrap(); let sidecar = rstore::recipients_dir(&ctx.store).join("alice.yaml"); assert!(sidecar.exists()); let meta: RecipientMeta = @@ -800,8 +772,8 @@ mod tests { #[test] fn list_recipients_wrapper_returns_sorted_entries() { let (_tmp, ctx) = mk_ctx(); - add_recipient(&ctx, "bob", AGE_KEY_2, None).unwrap(); - add_recipient(&ctx, "alice", AGE_KEY_1, Some("A".into())).unwrap(); + crate::cli::store_ops::recipient_add(&ctx, "bob", AGE_KEY_2, None).unwrap(); + crate::cli::store_ops::recipient_add(&ctx, "alice", AGE_KEY_1, Some("A".into())).unwrap(); let entries = list_recipients(&ctx).unwrap(); assert_eq!(entries.len(), 2); assert_eq!(entries[0].name, "alice"); @@ -812,8 +784,8 @@ mod tests { #[test] fn remove_recipient_wrapper_deletes_entry() { let (_tmp, ctx) = mk_ctx(); - add_recipient(&ctx, "alice", AGE_KEY_1, None).unwrap(); - remove_recipient(&ctx, "alice").unwrap(); + crate::cli::store_ops::recipient_add(&ctx, "alice", AGE_KEY_1, None).unwrap(); + crate::cli::store_ops::recipient_rm(&ctx, "alice").unwrap(); assert!(!rstore::recipients_dir(&ctx.store) .join("alice.pub") .exists()); diff --git a/rust/src/cli/resolver.rs b/rust/src/cli/resolver.rs index d203fc0..a657edc 100644 --- a/rust/src/cli/resolver.rs +++ b/rust/src/cli/resolver.rs @@ -90,30 +90,4 @@ impl SecretResolver { } } - /// Build resolver candidates for the store with real decrypted tags. - /// - /// Tag selectors and selector-valued aliases inside `outputs:` blocks only - /// resolve when candidates carry their tags, so `exec`, `generate`, and - /// `codegen` share this builder instead of passing `tags: vec![]` (which - /// made every `tag:` selector silently match nothing). - /// - /// Secrets the current identity cannot decrypt contribute no tags (they - /// simply won't match tag selectors), mirroring `ls --tag`. - pub fn resolve_candidates( - ctx: &Context, - ) -> Vec { - use crate::config::outputs::resolver::SecretCandidate; - - let identities = ctx.load_identities().unwrap_or_default(); - store::list_secrets(&ctx.store, None) - .unwrap_or_default() - .into_iter() - .map(|path| { - let tags = Self::resolve_with_identities(ctx, &path, &identities) - .map(|decoded| decoded.tags) - .unwrap_or_default(); - SecretCandidate { path, tags } - }) - .collect() - } } diff --git a/rust/src/cli/schema.rs b/rust/src/cli/schema.rs index 1dc4867..5dee5d0 100644 --- a/rust/src/cli/schema.rs +++ b/rust/src/cli/schema.rs @@ -250,6 +250,7 @@ mod tests { key_provider: crate::config::KeyProvider::default(), project_root: None, git: std::sync::Arc::new(crate::git::CliGitAdapter), + project_config_cell: Default::default(), }; cmd_refresh(&ctx).unwrap(); diff --git a/rust/src/cli/search.rs b/rust/src/cli/search.rs index 09476a5..8fdccbe 100644 --- a/rust/src/cli/search.rs +++ b/rust/src/cli/search.rs @@ -37,7 +37,7 @@ pub struct SearchArgs { } /// A single search result. -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, Default, PartialEq, Eq)] pub struct SearchResult { pub store: String, /// Filesystem path of the store that holds this result. Consumed by the diff --git a/rust/src/cli/set.rs b/rust/src/cli/set.rs index f3c0a60..1f8db23 100644 --- a/rust/src/cli/set.rs +++ b/rust/src/cli/set.rs @@ -107,7 +107,7 @@ pub fn set_plaintext( encrypt_and_write(ctx, path, &sv) } -fn encrypt_and_write(ctx: &Context, path: &str, sv: &SecretValue) -> Result { +pub(super) fn encrypt_and_write(ctx: &Context, path: &str, sv: &SecretValue) -> Result { let secret_ref = SecretRef::parse(path)?; let (effective_store, secret_path, recipients_path_override) = if secret_ref.is_qualified() { let resolved = secret_ref.resolve_store()?; diff --git a/rust/src/cli/store_ops.rs b/rust/src/cli/store_ops.rs new file mode 100644 index 0000000..833947f --- /dev/null +++ b/rust/src/cli/store_ops.rs @@ -0,0 +1,273 @@ +//! StoreOps — the mutation seam between presentation and the Store. +//! +//! One module owns the side-effect chain every Store mutation must run: +//! the append-only commit (`himitsu: {msg}` on success, `himitsu: FAILED: +//! {msg}: {err}` on failure, so `git status` is never left dirty), the +//! push (on success, unless opted out), and the completions-cache refresh +//! (on success, so the next tab-press sees the new path list). +//! +//! Two adapters sit in front of it: +//! - the **CLI dispatcher** calls [`finalize`] once per command (so batch +//! commands like `import` stay one commit), with the message from +//! `mutation_message`; +//! - **TUI views** call the silent mutation cores below ([`set_secret`], +//! [`delete_secret`], [`rekey`], [`join`], [`recipient_add`], +//! [`recipient_rm`]), which run the same chain per action. Cores never +//! touch stdin or stdout — errors come back as values, and ratatui owns +//! the screen. Generalizes the `add_core`/`rm_core` precedent (hm-by7). +//! +//! Global-config mutations (e.g. `remote add`) are not Store mutations — +//! they have no commit/cache chain and stay outside this module. + +use super::Context; +use crate::error::Result; +use crate::proto::SecretValue; + +/// Run the post-mutation side-effect chain for `result`. +/// +/// Commits on success **and** failure (the append-only invariant: a failed +/// mutation that left partial writes must still leave a clean tree, with +/// the `FAILED:` prefix recording what happened). Pushes and refreshes the +/// completions cache only on success. +pub fn finalize(ctx: &Context, msg: &str, no_push: bool, result: &Result) { + let final_msg = match result { + Ok(_) => format!("himitsu: {msg}"), + Err(e) => format!("himitsu: FAILED: {msg}: {e}"), + }; + let committed = ctx.commit(&final_msg); + if result.is_ok() && committed && !no_push { + ctx.push(); + } + // Keep the completions cache in sync after every successful mutation + // so the next tab-press sees the updated path list immediately. + if result.is_ok() && !ctx.store.as_os_str().is_empty() { + let _ = crate::completions_cache::refresh_store(&ctx.state_dir, &ctx.store); + } +} + +/// Run `mutation` under the chain: the mutation body executes, then +/// [`finalize`] runs regardless of outcome. +pub fn run_mutation( + ctx: &Context, + msg: &str, + no_push: bool, + mutation: impl FnOnce() -> Result, +) -> Result { + let result = mutation(); + finalize(ctx, msg, no_push, &result); + result +} + +/// Create or overwrite a Secret: validate env-key metadata, encrypt to the +/// effective store's recipients, write, then run the chain. Returns the +/// normalized secret path. +pub fn set_secret(ctx: &Context, path: &str, sv: &SecretValue) -> Result { + run_mutation(ctx, &format!("set {path}"), false, || { + if !sv.env_key.is_empty() { + super::set::validate_env_key(&sv.env_key)?; + } + super::set::encrypt_and_write(ctx, path, sv) + }) +} + +/// Delete a Secret from the active store, then run the chain. +pub fn delete_secret(ctx: &Context, path: &str) -> Result<()> { + run_mutation(ctx, &format!("delete {path}"), false, || { + crate::remote::store::delete_secret(&ctx.store, path) + }) +} + +/// Re-encrypt secrets for the store's current recipient list (optionally +/// narrowed to a path prefix), then run the chain. Returns the number of +/// secrets rekeyed. +pub fn rekey(ctx: &Context, path_prefix: Option<&str>) -> Result { + let msg = match path_prefix { + Some(p) => format!("rekey {p}"), + None => "rekey".to_string(), + }; + run_mutation(ctx, &msg, false, || { + super::rekey::rekey_store(ctx, path_prefix) + }) +} + +/// Join the active store as a recipient (idempotent), then run the chain. +pub fn join(ctx: &Context) -> Result { + run_mutation(ctx, "join", false, || super::join::join_core(ctx, None)) +} + +/// Add a recipient by explicit age public key, then run the chain. An +/// empty/whitespace description is normalized to `None` so the core never +/// writes an empty sidecar. +pub fn recipient_add( + ctx: &Context, + name: &str, + age_key: &str, + description: Option, +) -> Result<()> { + run_mutation(ctx, &format!("recipient add {name}"), false, || { + let description = description.and_then(|d| { + let t = d.trim().to_string(); + (!t.is_empty()).then_some(t) + }); + super::recipient::add_core(ctx, name, Some(age_key), description).map(|_missing| ()) + }) +} + +/// Remove a recipient by name, then run the chain. +pub fn recipient_rm(ctx: &Context, name: &str) -> Result<()> { + run_mutation(ctx, &format!("recipient rm {name}"), false, || { + super::recipient::rm_core(ctx, name) + }) +} + +#[cfg(test)] +mod tests { + use std::path::Path; + use std::sync::Arc; + + use super::*; + use crate::crypto::age; + use crate::error::HimitsuError; + use crate::remote::store as rstore; + + /// A Context over a tempdir store that is a real git repo, with one age + /// identity on disk and the store's recipients configured. + fn test_ctx() -> (tempfile::TempDir, Context) { + let tmp = tempfile::tempdir().unwrap(); + let data_dir = tmp.path().join("data"); + let state_dir = tmp.path().join("state"); + let store = tmp.path().join("store"); + std::fs::create_dir_all(&data_dir).unwrap(); + std::fs::create_dir_all(&state_dir).unwrap(); + std::fs::create_dir_all(rstore::recipients_dir(&store)).unwrap(); + std::fs::create_dir_all(rstore::secrets_dir(&store)).unwrap(); + + let (secret, public) = age::keygen(); + std::fs::write( + data_dir.join("key"), + format!("# public key: {public}\n{secret}\n"), + ) + .unwrap(); + std::fs::write( + rstore::recipients_dir(&store).join("me.pub"), + format!("{public}\n"), + ) + .unwrap(); + + crate::git::run(&["init", "-q"], &store).unwrap(); + crate::git::run(&["config", "user.email", "t@t"], &store).unwrap(); + crate::git::run(&["config", "user.name", "t"], &store).unwrap(); + + let ctx = Context { + data_dir, + state_dir, + store, + recipients_path: None, + key_provider: crate::config::KeyProvider::default(), + project_root: None, + git: Arc::new(crate::git::CliGitAdapter), + project_config_cell: Default::default(), + }; + (tmp, ctx) + } + + fn last_commit_subject(store: &Path) -> String { + crate::git::run(&["log", "--format=%s", "-1"], store) + .unwrap() + .trim() + .to_string() + } + + #[test] + fn set_secret_runs_the_full_chain() { + let (_tmp, ctx) = test_ctx(); + + let sv = SecretValue { + data: b"v1".to_vec(), + ..Default::default() + }; + let path = set_secret(&ctx, "prod/api-key", &sv).unwrap(); + assert_eq!(path, "prod/api-key"); + + // Mutation landed and is readable back through the resolver. + let decoded = crate::cli::resolver::SecretResolver::resolve(&ctx, "prod/api-key").unwrap(); + assert_eq!(decoded.data, b"v1"); + + // Append-only commit with the canonical message; tree left clean. + assert_eq!(last_commit_subject(&ctx.store), "himitsu: set prod/api-key"); + let status = crate::git::run(&["status", "--porcelain"], &ctx.store).unwrap(); + assert!(status.trim().is_empty(), "tree dirty: {status}"); + + // Completions cache refreshed — the new path is immediately visible. + let hits = + crate::completions_cache::lookup(&ctx.state_dir, std::slice::from_ref(&ctx.store), "prod") + .unwrap(); + assert!(hits.iter().any(|h| h == "prod/api-key"), "{hits:?}"); + } + + #[test] + fn set_secret_validates_env_key_metadata() { + let (_tmp, ctx) = test_ctx(); + let sv = SecretValue { + data: b"v".to_vec(), + env_key: "1NOT VALID".to_string(), + ..Default::default() + }; + assert!(set_secret(&ctx, "prod/x", &sv).is_err()); + } + + #[test] + fn delete_secret_commits_and_refreshes() { + let (_tmp, ctx) = test_ctx(); + let sv = SecretValue { + data: b"v".to_vec(), + ..Default::default() + }; + set_secret(&ctx, "prod/gone", &sv).unwrap(); + + delete_secret(&ctx, "prod/gone").unwrap(); + + assert_eq!(last_commit_subject(&ctx.store), "himitsu: delete prod/gone"); + let status = crate::git::run(&["status", "--porcelain"], &ctx.store).unwrap(); + assert!(status.trim().is_empty(), "tree dirty: {status}"); + let hits = + crate::completions_cache::lookup(&ctx.state_dir, std::slice::from_ref(&ctx.store), "prod") + .unwrap(); + assert!(!hits.iter().any(|h| h == "prod/gone"), "{hits:?}"); + } + + #[test] + fn failed_mutation_commits_the_failed_marker() { + let (_tmp, ctx) = test_ctx(); + + // A mutation that writes something, then fails: the chain must + // still commit, with the FAILED prefix recording the partial state. + let result: Result<()> = run_mutation(&ctx, "test-op", true, || { + std::fs::write( + rstore::secrets_dir(&ctx.store).join("partial.age"), + b"junk", + ) + .unwrap(); + Err(HimitsuError::NotSupported("boom".into())) + }); + assert!(result.is_err()); + + let subject = last_commit_subject(&ctx.store); + assert!( + subject.starts_with("himitsu: FAILED: test-op"), + "got: {subject}" + ); + let status = crate::git::run(&["status", "--porcelain"], &ctx.store).unwrap(); + assert!(status.trim().is_empty(), "tree dirty: {status}"); + } + + #[test] + fn join_core_outcomes_are_silent_values() { + let (_tmp, ctx) = test_ctx(); + // The fixture's own key is already a recipient ("me.pub" holds it). + match join(&ctx).unwrap() { + super::super::join::JoinOutcome::AlreadyRecipient => {} + other => panic!("expected AlreadyRecipient, got {other:?}"), + } + } +} diff --git a/rust/src/cli/sync.rs b/rust/src/cli/sync.rs index 4250b4d..6faa73f 100644 --- a/rust/src/cli/sync.rs +++ b/rust/src/cli/sync.rs @@ -47,6 +47,7 @@ pub fn run(args: SyncArgs, ctx: &Context) -> Result<()> { key_provider: ctx.key_provider.clone(), project_root: ctx.project_root.clone(), git: ctx.git.clone(), + project_config_cell: ctx.project_config_cell.clone(), }; // Commit any pre-existing pending changes (e.g. from a prior sync that diff --git a/rust/src/completions_cache.rs b/rust/src/completions_cache.rs index d8b8caa..45e7f95 100644 --- a/rust/src/completions_cache.rs +++ b/rust/src/completions_cache.rs @@ -50,16 +50,18 @@ pub fn refresh_store(state_dir: &Path, store: &Path) -> rusqlite::Result let secrets_dir = store.join(".himitsu").join("secrets"); let current_mtime = max_mtime_recursive(&secrets_dir); - // Check whether the tree has changed since the last cache build. - let stored_mtime: i64 = conn + // Check whether the tree has changed since the last cache build. The + // stored value is the bit-cast fingerprint; a missing row means "never + // built" and forces a rebuild. + let stored_fingerprint: Option = conn .query_row( "SELECT mtime FROM store_info WHERE store_path = ?1", params![store_key], |row| row.get(0), ) - .unwrap_or(-1); + .ok(); - if stored_mtime >= 0 && stored_mtime as u64 == current_mtime { + if stored_fingerprint == Some(current_mtime as i64) { // Tree unchanged — return the cached count without touching paths. let count: i64 = conn .query_row( @@ -199,35 +201,47 @@ fn ensure_schema(conn: &Connection) -> rusqlite::Result<()> { ) } -/// Recursive max-mtime over a directory tree. Returns 0 if the directory -/// doesn't exist or mtime can't be read (drives a full rebuild on next call). +/// Change fingerprint of a directory tree: max mtime (nanosecond precision) +/// and entry count, hashed together order-independently. Either a content +/// change or a deletion produces a new fingerprint — a plain +/// seconds-granularity max-mtime missed deletions that happened within the +/// same second as the last write. Returns 0 if the directory doesn't exist +/// (drives a full rebuild on next call). fn max_mtime_recursive(dir: &Path) -> u64 { - if !dir.exists() { - return 0; - } - let mut max = 0u64; - let Ok(entries) = std::fs::read_dir(dir) else { - return 0; - }; - for entry in entries.flatten() { - let Ok(meta) = entry.metadata() else { continue }; - if let Ok(modified) = meta.modified() { - let secs = modified - .duration_since(std::time::UNIX_EPOCH) - .unwrap_or_default() - .as_secs(); - if secs > max { - max = secs; + use std::hash::{Hash, Hasher}; + + fn walk(dir: &Path, max_nanos: &mut u128, count: &mut u64) { + let Ok(entries) = std::fs::read_dir(dir) else { + return; + }; + for entry in entries.flatten() { + let Ok(meta) = entry.metadata() else { continue }; + *count += 1; + if let Ok(modified) = meta.modified() { + let nanos = modified + .duration_since(std::time::UNIX_EPOCH) + .unwrap_or_default() + .as_nanos(); + if nanos > *max_nanos { + *max_nanos = nanos; + } } - } - if meta.is_dir() { - let child = max_mtime_recursive(&entry.path()); - if child > max { - max = child; + if meta.is_dir() { + walk(&entry.path(), max_nanos, count); } } } - max + + if !dir.exists() { + return 0; + } + let mut max_nanos = 0u128; + let mut count = 0u64; + walk(dir, &mut max_nanos, &mut count); + let mut hasher = std::collections::hash_map::DefaultHasher::new(); + max_nanos.hash(&mut hasher); + count.hash(&mut hasher); + hasher.finish() } // ── Tests ──────────────────────────────────────────────────────────────────── diff --git a/rust/src/config/mod.rs b/rust/src/config/mod.rs index 2ec8690..5278fee 100644 --- a/rust/src/config/mod.rs +++ b/rust/src/config/mod.rs @@ -586,13 +586,19 @@ pub fn find_project_config_from(start: &Path) -> Option { None } -/// Load and parse the first project config found by [`find_project_config`]. +/// Load and parse the first project config found by [`find_project_config`] +/// (cwd walk). +/// +/// Internal to the Context machinery: commands and views must go through +/// [`Context::project_config`](crate::cli::Context::project_config), which +/// owns the "which project config applies" decision (explicit `--project` +/// root vs cwd walk) and memoizes the result. /// /// Returns `Ok(Some((config, path)))` if a config file exists and parses /// successfully, `Ok(None)` if no config file is found, or `Err` if a config /// file exists but is malformed YAML. A legacy `envs:` key is tolerated (it is /// captured and a migration warning is emitted), not a parse error. -pub fn load_project_config() -> Result> { +pub(crate) fn load_project_config() -> Result> { let Some(path) = find_project_config() else { return Ok(None); }; @@ -603,10 +609,13 @@ pub fn load_project_config() -> Result> { } /// Load and parse the first project config found by walking upward from -/// `start`. Unlike [`load_project_config`], parse errors are surfaced as -/// `Err` so callers in explicit project mode can fail loudly when the -/// config file is present but malformed. -pub fn load_project_config_from(start: &Path) -> Result> { +/// `start`. Same contract as [`load_project_config`], with an explicit +/// starting point instead of a cwd walk. +/// +/// Internal: the sanctioned direct callers are `migrate`'s multi-root scan +/// and the Context construction/memoization machinery. Commands and views +/// must use [`Context::project_config`](crate::cli::Context::project_config). +pub(crate) fn load_project_config_from(start: &Path) -> Result> { let Some(path) = find_project_config_from(start) else { return Ok(None); }; diff --git a/rust/src/tui/app.rs b/rust/src/tui/app.rs index e6c3137..465d481 100644 --- a/rust/src/tui/app.rs +++ b/rust/src/tui/app.rs @@ -606,28 +606,33 @@ impl App { } /// Build a [`HelpView`] populated with entries for whichever view is - /// currently active. + /// currently active. Views with rebindable actions derive their rows + /// from the live keymap, so a user rebind shows up in help immediately. fn help_for_current_view(&self) -> HelpView { match &self.view { - View::Search(_) => HelpView::new(SearchView::help_entries(), SearchView::help_title()), + View::Search(_) => HelpView::new( + SearchView::help_entries(&self.keymap), + SearchView::help_title(), + ), View::SecretViewer(_) => HelpView::new( - SecretViewerView::help_entries(), + SecretViewerView::help_entries(&self.keymap), SecretViewerView::help_title(), ), - View::NewSecret(_) => { - HelpView::new(NewSecretView::help_entries(), NewSecretView::help_title()) - } + View::NewSecret(_) => HelpView::new( + NewSecretView::help_entries(&self.keymap), + NewSecretView::help_title(), + ), View::Outputs(_) => { - HelpView::new(OutputsView::help_entries(), OutputsView::help_title()) + HelpView::from_static(OutputsView::help_entries(), OutputsView::help_title()) } View::RemoteAdd(_) => { - HelpView::new(RemoteAddView::help_entries(), RemoteAddView::help_title()) + HelpView::from_static(RemoteAddView::help_entries(), RemoteAddView::help_title()) } - View::RecipientList(_) => HelpView::new( + View::RecipientList(_) => HelpView::from_static( RecipientListView::help_entries(), RecipientListView::help_title(), ), - View::RecipientAdd(_) => HelpView::new( + View::RecipientAdd(_) => HelpView::from_static( RecipientAddView::help_entries(), RecipientAddView::help_title(), ), @@ -654,6 +659,7 @@ fn clone_ctx(ctx: &Context) -> Context { key_provider: ctx.key_provider.clone(), project_root: ctx.project_root.clone(), git: ctx.git.clone(), + project_config_cell: ctx.project_config_cell.clone(), } } diff --git a/rust/src/tui/harness.rs b/rust/src/tui/harness.rs index 4ceded5..220420f 100644 --- a/rust/src/tui/harness.rs +++ b/rust/src/tui/harness.rs @@ -240,6 +240,7 @@ mod tests { key_provider: crate::config::KeyProvider::default(), project_root: None, git: std::sync::Arc::new(crate::git::CliGitAdapter), + project_config_cell: Default::default(), }; Self { diff --git a/rust/src/tui/keymap.rs b/rust/src/tui/keymap.rs index d2e472b..a816243 100644 --- a/rust/src/tui/keymap.rs +++ b/rust/src/tui/keymap.rs @@ -154,6 +154,9 @@ impl fmt::Display for KeyBinding { fn code_to_string(code: KeyCode) -> String { match code { + // The named form roundtrips: chord steps are whitespace-separated in + // the config format, so a literal ' ' would be unparseable. + KeyCode::Char(' ') => "space".to_string(), KeyCode::Char(c) => c.to_string(), KeyCode::Enter => "enter".to_string(), KeyCode::Esc => "esc".to_string(), @@ -509,6 +512,12 @@ pub enum KeyAction { CollapsePaths, /// In the search view: expand all secret paths back to full depth. ExpandPaths, + /// In the search view: toggle the secret-ref autocomplete popup. + ToggleAutocomplete, + /// In the search view: refine the query to the selected row's tag. + RefineTag, + /// In the search view: sort by the selected results column. + SortColumn, Reveal, CopyValue, @@ -565,13 +574,23 @@ pub struct KeyMap { /// secret without putting plaintext on the clipboard. Default: `Y` /// (Shift+y) — symmetric with the viewer. pub copy_ref_selected: Vec, - pub envs: Vec, + /// Open the outputs browser (default: `Shift+E`). The serde alias + /// accepts the pre-rename `envs` config key, so existing user keymaps + /// keep working. + #[serde(alias = "envs")] + pub outputs: Vec, /// Collapse all secret paths to top-level folders (default: `Ctrl+-`, /// leader fallback `Ctrl+x -`). pub collapse_paths: Vec, /// Expand all secret paths to full depth (default: `Ctrl++` / `Ctrl+=`, /// leader fallback `Ctrl+x +`). pub expand_paths: Vec, + /// Toggle the secret-ref autocomplete popup (default: `Ctrl+Space`). + pub toggle_autocomplete: Vec, + /// Refine the query to the selected row's tag (default: `Ctrl+T`). + pub refine_tag: Vec, + /// Sort by the selected results column (default: `Ctrl+O`). + pub sort_column: Vec, // ── Secret viewer ──────────────────────────────────────────────── pub reveal: Vec, @@ -617,9 +636,12 @@ impl Default for KeyMap { switch_store: vec![ctrl('s')], copy_selected: vec![ctrl('y')], copy_ref_selected: vec![shift_char('y')], - envs: vec![shift_char('e')], + outputs: vec![shift_char('e')], collapse_paths: vec![ctrl('-'), leader_x('-')], expand_paths: vec![ctrl('+'), ctrl('='), leader_x('+')], + toggle_autocomplete: vec![ctrl(' ')], + refine_tag: vec![ctrl('t')], + sort_column: vec![ctrl('o')], reveal: vec![bare(KeyCode::Char('r'))], copy_value: vec![bare(KeyCode::Char('y'))], @@ -637,35 +659,262 @@ impl Default for KeyMap { } } +// ── KeyRegistry ──────────────────────────────────────────────────────────── + +/// Which view's help screen lists a [`KeyAction`]. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Scope { + /// Available everywhere (quit, help). + Global, + /// Search view. + Search, + /// Secret viewer. + Viewer, + /// New-secret form. + NewSecretForm, +} + +/// One registry row: everything every surface needs to know about a +/// [`KeyAction`] — its config field, its help text, which view's help +/// screen lists it, and the command-palette entry that shares it. +pub struct Row { + /// Accessor for the user-configurable chord list backing this action. + pub field: fn(&KeyMap) -> &Vec, + /// Help-screen description. + pub help: &'static str, + /// Which view's help screen lists this action. + pub scope: Scope, + /// The command-palette command sharing this action, if any. + pub palette: Option, +} + +impl KeyAction { + /// Every action, in help-screen display order (grouped by scope). Used + /// to derive [`KeyMap::entries`] — an action missing here simply never + /// dispatches, which is loud, unlike the silent drift of the old + /// hand-maintained table. + pub const ALL: [KeyAction; 24] = [ + // Global + KeyAction::Help, + KeyAction::Quit, + // Search view + KeyAction::CommandPalette, + KeyAction::NewSecret, + KeyAction::SwitchStore, + KeyAction::CopySelected, + KeyAction::CopyRefSelected, + KeyAction::Outputs, + KeyAction::CollapsePaths, + KeyAction::ExpandPaths, + KeyAction::ToggleAutocomplete, + KeyAction::RefineTag, + KeyAction::SortColumn, + // Secret viewer + KeyAction::Reveal, + KeyAction::CopyValue, + KeyAction::CopyRef, + KeyAction::Rekey, + KeyAction::Edit, + KeyAction::Delete, + KeyAction::Back, + // New-secret form + KeyAction::SaveSecret, + KeyAction::NextField, + KeyAction::PrevField, + KeyAction::Cancel, + ]; +} + +/// The single source of truth tying a [`KeyAction`] to its config field, +/// help text, scope, and palette link. The exhaustive match means adding a +/// `KeyAction` variant without a row is a **compile error** — this replaces +/// the hand-synced `entries()` table, the per-view hardcoded help strings, +/// and the palette's hand-maintained action map. +pub fn row(action: KeyAction) -> Row { + use crate::tui::views::command_palette::Command; + match action { + KeyAction::Quit => Row { + field: |km| &km.quit, + help: "quit", + scope: Scope::Global, + palette: Some(Command::Quit), + }, + KeyAction::Help => Row { + field: |km| &km.help, + help: "toggle this help", + scope: Scope::Global, + palette: Some(Command::Help), + }, + KeyAction::CommandPalette => Row { + field: |km| &km.command_palette, + help: "open command palette", + scope: Scope::Search, + palette: None, + }, + KeyAction::NewSecret => Row { + field: |km| &km.new_secret, + help: "new secret", + scope: Scope::Search, + palette: Some(Command::NewSecret), + }, + KeyAction::SwitchStore => Row { + field: |km| &km.switch_store, + help: "switch store", + scope: Scope::Search, + palette: Some(Command::SwitchStore), + }, + KeyAction::CopySelected => Row { + field: |km| &km.copy_selected, + help: "copy selection to clipboard", + scope: Scope::Search, + palette: None, + }, + KeyAction::CopyRefSelected => Row { + field: |km| &km.copy_ref_selected, + help: "copy `himitsu read ` command", + scope: Scope::Search, + palette: None, + }, + KeyAction::Outputs => Row { + field: |km| &km.outputs, + help: "browse outputs", + scope: Scope::Search, + palette: Some(Command::Outputs), + }, + KeyAction::CollapsePaths => Row { + field: |km| &km.collapse_paths, + help: "collapse paths to top-level folders", + scope: Scope::Search, + palette: None, + }, + KeyAction::ExpandPaths => Row { + field: |km| &km.expand_paths, + help: "expand paths to full depth", + scope: Scope::Search, + palette: None, + }, + KeyAction::ToggleAutocomplete => Row { + field: |km| &km.toggle_autocomplete, + help: "toggle ref autocomplete", + scope: Scope::Search, + palette: None, + }, + KeyAction::RefineTag => Row { + field: |km| &km.refine_tag, + help: "refine to selected tag", + scope: Scope::Search, + palette: None, + }, + KeyAction::SortColumn => Row { + field: |km| &km.sort_column, + help: "sort selected column", + scope: Scope::Search, + palette: None, + }, + KeyAction::Reveal => Row { + field: |km| &km.reveal, + help: "reveal / hide value", + scope: Scope::Viewer, + palette: None, + }, + KeyAction::CopyValue => Row { + field: |km| &km.copy_value, + help: "copy value to clipboard", + scope: Scope::Viewer, + palette: None, + }, + KeyAction::CopyRef => Row { + field: |km| &km.copy_ref, + help: "copy `himitsu read ` command", + scope: Scope::Viewer, + palette: None, + }, + KeyAction::Rekey => Row { + field: |km| &km.rekey, + help: "rekey for current recipients", + scope: Scope::Viewer, + palette: None, + }, + KeyAction::Edit => Row { + field: |km| &km.edit, + help: "edit value + metadata in $EDITOR", + scope: Scope::Viewer, + palette: None, + }, + KeyAction::Delete => Row { + field: |km| &km.delete, + help: "delete secret (with confirm)", + scope: Scope::Viewer, + palette: None, + }, + KeyAction::Back => Row { + field: |km| &km.back, + help: "back", + scope: Scope::Viewer, + palette: None, + }, + KeyAction::SaveSecret => Row { + field: |km| &km.save_secret, + help: "save from any field", + scope: Scope::NewSecretForm, + palette: None, + }, + KeyAction::NextField => Row { + field: |km| &km.next_field, + help: "next field (wraps)", + scope: Scope::NewSecretForm, + palette: None, + }, + KeyAction::PrevField => Row { + field: |km| &km.prev_field, + help: "previous field (wraps)", + scope: Scope::NewSecretForm, + palette: None, + }, + KeyAction::Cancel => Row { + field: |km| &km.cancel, + help: "cancel", + scope: Scope::NewSecretForm, + palette: None, + }, + } +} + +/// Live help rows for one scope: `(chords, description)` with the chord +/// display rendered from the CURRENT keymap, so user rebinds show up in +/// every help screen automatically. +pub fn help_rows(keymap: &KeyMap, scope: Scope) -> Vec<(String, String)> { + KeyAction::ALL + .into_iter() + .filter(|a| row(*a).scope == scope) + .map(|a| (chords_display(keymap, a), row(a).help.to_string())) + .collect() +} + +/// One live help row for a single action — for views that compose their +/// help screens from individual rows rather than a whole scope. +pub fn help_row(keymap: &KeyMap, action: KeyAction) -> (String, String) { + (chords_display(keymap, action), row(action).help.to_string()) +} + +/// Human-facing display of every chord bound to `action`, joined with +/// ` / ` — e.g. `ctrl-- / ctrl-x -`. The keymap's `+` separator renders +/// as `-` to match the command palette's established style. +pub fn chords_display(keymap: &KeyMap, action: KeyAction) -> String { + keymap + .chords_for(action) + .iter() + .map(|c| c.to_string().replace('+', "-")) + .collect::>() + .join(" / ") +} + impl KeyMap { - /// `(action, chords)` pairs across every keymap field. Stack-allocated - /// so the dispatcher (called per keystroke) doesn't churn the heap. - /// When you add a new `KeyAction`, append it here and bump the array - /// length. - fn entries(&self) -> [(KeyAction, &Vec); 21] { - [ - (KeyAction::Quit, &self.quit), - (KeyAction::Help, &self.help), - (KeyAction::CommandPalette, &self.command_palette), - (KeyAction::NewSecret, &self.new_secret), - (KeyAction::SwitchStore, &self.switch_store), - (KeyAction::CopySelected, &self.copy_selected), - (KeyAction::CopyRefSelected, &self.copy_ref_selected), - (KeyAction::Outputs, &self.envs), - (KeyAction::CollapsePaths, &self.collapse_paths), - (KeyAction::ExpandPaths, &self.expand_paths), - (KeyAction::Reveal, &self.reveal), - (KeyAction::CopyValue, &self.copy_value), - (KeyAction::CopyRef, &self.copy_ref), - (KeyAction::Rekey, &self.rekey), - (KeyAction::Edit, &self.edit), - (KeyAction::Delete, &self.delete), - (KeyAction::Back, &self.back), - (KeyAction::SaveSecret, &self.save_secret), - (KeyAction::NextField, &self.next_field), - (KeyAction::PrevField, &self.prev_field), - (KeyAction::Cancel, &self.cancel), - ] + /// `(action, chords)` pairs across every keymap field, derived from + /// [`KeyAction::ALL`] and the registry [`row`]s — there is no second + /// table to keep in sync. + fn entries(&self) -> [(KeyAction, &Vec); KeyAction::ALL.len()] { + KeyAction::ALL.map(|action| (action, (row(action).field)(self))) } /// Single-step direct lookup: find the action whose binding list @@ -697,18 +946,11 @@ impl KeyMap { .find(|&action| self.chords_for(action).matches(key)) } - /// Borrow the chord list registered for a given action. Looked up - /// linearly through [`Self::entries`] — ~19 entries, called per - /// keystroke; the cost is dwarfed by terminal redraw. + /// Borrow the chord list registered for a given action — a direct + /// registry-field access; total by construction (the registry match is + /// exhaustive over [`KeyAction`]). pub fn chords_for(&self, action: KeyAction) -> &Vec { - for (a, chords) in self.entries() { - if a == action { - return chords; - } - } - // entries() covers every variant of KeyAction, so this is - // unreachable in practice. - unreachable!("KeyMap::entries missing variant {action:?}") + (row(action).field)(self) } /// Drive the leader-key state machine. @@ -786,47 +1028,77 @@ mod tests { } #[test] - fn entries_cover_every_key_action_variant() { - // entries() returns a fixed-size array; chords_for() relies on it - // covering every KeyAction. If a variant is added without being - // appended to entries(), chords_for() would hit its unreachable!() - // arm. Exercise every variant through chords_for to guarantee - // coverage and catch drift at test time rather than runtime. + fn registry_covers_every_key_action_variant() { + // Row coverage is a COMPILE error now (the registry match is + // exhaustive); what's left to pin at test time is that ALL lists + // every variant exactly once (an action missing from ALL never + // dispatches), that every action has a non-empty default binding, + // and that every row carries help text. let km = KeyMap::default(); - for action in [ - KeyAction::Quit, - KeyAction::Help, - KeyAction::CommandPalette, - KeyAction::NewSecret, - KeyAction::SwitchStore, - KeyAction::CopySelected, - KeyAction::CopyRefSelected, - KeyAction::Outputs, - KeyAction::CollapsePaths, - KeyAction::ExpandPaths, - KeyAction::Reveal, - KeyAction::CopyValue, - KeyAction::CopyRef, - KeyAction::Rekey, - KeyAction::Edit, - KeyAction::Delete, - KeyAction::Back, - KeyAction::SaveSecret, - KeyAction::NextField, - KeyAction::PrevField, - KeyAction::Cancel, - ] { - // Must not panic (chords_for has an unreachable! for missing - // variants) — every action is registered in entries(). - let _ = km.chords_for(action); + for action in KeyAction::ALL { + assert!( + !km.chords_for(action).is_empty(), + "{action:?} has no default binding" + ); + assert!(!row(action).help.is_empty(), "{action:?} has empty help"); } - assert_eq!( - km.entries().len(), - 21, - "entries array length tracks variants" + // Duplicate detection without Hash: pairwise. + for (i, a) in KeyAction::ALL.iter().enumerate() { + for b in &KeyAction::ALL[i + 1..] { + assert_ne!(a, b, "duplicate entry in KeyAction::ALL"); + } + } + assert_eq!(km.entries().len(), KeyAction::ALL.len()); + } + + #[test] + fn help_rows_render_live_bindings() { + // Rebinding an action must change the help row — help screens can't + // lie. Default first: + let km = KeyMap::default(); + let rows = help_rows(&km, Scope::Search); + assert!( + rows.iter() + .any(|(k, d)| k == "ctrl-t" && d == "refine to selected tag"), + "{rows:?}" + ); + + // Rebind and confirm the row follows. + let remapped = KeyMap { + refine_tag: vec![KeyChord::single(KeyBinding::ctrl('g'))], + ..KeyMap::default() + }; + let rows = help_rows(&remapped, Scope::Search); + assert!( + rows.iter() + .any(|(k, d)| k == "ctrl-g" && d == "refine to selected tag"), + "{rows:?}" ); } + #[test] + fn legacy_envs_config_key_still_binds_outputs() { + // The field was renamed envs -> outputs; the serde alias must keep + // pre-rename user keymaps working. + let yaml = r#" +envs: ["ctrl+l"] +"#; + let km: KeyMap = serde_yaml::from_str(yaml).unwrap(); + assert!(km + .outputs + .matches(&key(KeyCode::Char('l'), KeyModifiers::CONTROL))); + } + + #[test] + fn palette_links_resolve_through_registry() { + use crate::tui::views::command_palette::Command; + // The palette's key_action() derives from registry rows; spot-check + // the link survives in both directions. + assert_eq!(Command::Outputs.key_action(), Some(KeyAction::Outputs)); + assert_eq!(row(KeyAction::Outputs).palette, Some(Command::Outputs)); + assert_eq!(Command::Import.key_action(), None); + } + #[test] fn fold_actions_have_default_and_leader_bindings() { let km = KeyMap::default(); diff --git a/rust/src/tui/mod.rs b/rust/src/tui/mod.rs index ecd48e7..9b47db5 100644 --- a/rust/src/tui/mod.rs +++ b/rust/src/tui/mod.rs @@ -13,6 +13,7 @@ pub mod forms; mod harness; mod hint; mod icons; +mod model; pub mod keymap; pub mod layout; mod terminal; @@ -97,6 +98,7 @@ pub fn run_init_flow() -> Result<()> { key_provider: crate::config::KeyProvider::default(), project_root: None, git: std::sync::Arc::new(crate::git::CliGitAdapter), + project_config_cell: Default::default(), }; let result = init::run(args, &ctx); @@ -134,6 +136,7 @@ pub fn run_init_flow() -> Result<()> { key_provider: cfg.key_provider, project_root: None, git: std::sync::Arc::new(crate::git::CliGitAdapter), + project_config_cell: Default::default(), }; if !should_continue_to_dashboard_after_init(&ctx.store) { return Ok(()); @@ -190,6 +193,7 @@ mod tests { key_provider: crate::config::KeyProvider::default(), project_root: None, git: std::sync::Arc::new(crate::git::CliGitAdapter), + project_config_cell: Default::default(), } } diff --git a/rust/src/tui/model/mod.rs b/rust/src/tui/model/mod.rs new file mode 100644 index 0000000..6a68e65 --- /dev/null +++ b/rust/src/tui/model/mod.rs @@ -0,0 +1,8 @@ +//! Pure state modules backing the result-list views. +//! +//! Graduated out of the search view (2026-06-09 architecture review): +//! results in → rows out, no ratatui imports, unit-testable without a +//! terminal. Views own rendering; these modules own the row model. + +pub mod path_folding; +pub mod result_sort; diff --git a/rust/src/tui/model/path_folding.rs b/rust/src/tui/model/path_folding.rs new file mode 100644 index 0000000..b1a71ee --- /dev/null +++ b/rust/src/tui/model/path_folding.rs @@ -0,0 +1,279 @@ +//! PathFolding — collapse/expand of secret paths into prefix groups. +//! +//! Pure state module: a flat result list in, display rows out. Owns the +//! store-bucket partitioning, the prefix grouping (a "group" is any +//! top-level path segment with ≥ 2 leaves), and the folded/unfolded row +//! shapes. No ratatui imports — views own rendering. + +use crate::cli::search::SearchResult; + +use super::result_sort::{compare_strings, sort_results, SearchColumn, SortDirection, SortState}; + +/// A row in the rendered results list. `Store` headers group secrets by +/// origin (`org/repo` slug or local path) and are never selectable; +/// navigation steps over them. `FoldedGroup` rows appear only in folded +/// mode, one per top-level path prefix shared by ≥ 2 secrets — they +/// collapse the group's leaves into a single selectable row that expands +/// when the user unfolds. +#[derive(Debug, Clone)] +pub enum Row { + Store { + name: String, + count: usize, + }, + FoldedGroup { + /// Top-level path segment shared by the collapsed leaves. + prefix: String, + /// Number of leaves under this prefix. + count: usize, + /// Indentation depth (matches what its children would have if + /// expanded). 0 in single-store mode, 1 under a `Store` header. + indent: usize, + }, + Secret { + result: SearchResult, + /// Indentation depth in list-item cells (2 spaces per level). + /// 0 in single-store mode, 1 under a `Store` header. + indent: usize, + /// Top-level path segment when this secret shares a prefix with + /// ≥ 1 sibling in the same store. The renderer paints this segment + /// with a subtle accent so the visual grouping survives without a + /// separate header row. `None` for singletons. + shared_prefix: Option, + }, +} + +/// Group a flat list of results into rows. +/// +/// When results span **multiple stores**, rows are partitioned per-store with +/// a `Store` header row per bucket; within each bucket we apply path-prefix +/// grouping. When only one store is present we fall back to the single-store +/// layout (no store header). +/// +/// A "group" is any top-level path segment that contains ≥ 2 leaves. In +/// folded mode each such group collapses to a single `FoldedGroup` row; in +/// unfolded mode the leaves render inline with their shared prefix tagged so +/// the renderer can paint it in a subtle accent. Singletons render the same +/// in both modes. The active sort column controls ordering inside each store; +/// store headers stay grouped for readability. +pub fn build_rows(results: &[SearchResult], folded: bool, sort_state: SortState) -> Vec { + use std::collections::BTreeMap; + + let mut by_store: BTreeMap> = BTreeMap::new(); + for r in results { + by_store.entry(r.store.clone()).or_default().push(r.clone()); + } + + let multi_store = by_store.len() > 1; + let mut rows = Vec::new(); + let mut store_names: Vec = by_store.keys().cloned().collect(); + if sort_state.column == SearchColumn::Store && sort_state.direction == SortDirection::Desc { + store_names.reverse(); + } + for store_name in store_names { + let bucket = by_store.remove(&store_name).unwrap_or_default(); + if multi_store { + rows.push(Row::Store { + name: store_name.clone(), + count: bucket.len(), + }); + } + append_prefix_grouped_rows(&mut rows, bucket, multi_store, folded, sort_state); + } + rows +} + +/// Append `bucket` rows to `rows` applying path-prefix grouping. +/// +/// `under_store_header` adds one level of indent so each store's children +/// visually nest. `folded` collapses ≥ 2-leaf groups into `FoldedGroup` rows. +fn append_prefix_grouped_rows( + rows: &mut Vec, + mut bucket: Vec, + under_store_header: bool, + folded: bool, + sort_state: SortState, +) { + use std::collections::HashMap; + + let store_indent: usize = if under_store_header { 1 } else { 0 }; + let bucket_sort = if sort_state.column == SearchColumn::Store { + SortState { + column: SearchColumn::Path, + direction: SortDirection::Asc, + } + } else { + sort_state + }; + sort_results(&mut bucket, bucket_sort); + + let mut order: Vec = Vec::new(); + let mut groups: HashMap> = HashMap::new(); + for r in bucket { + let prefix = prefix_of(&r.path).to_string(); + if !groups.contains_key(&prefix) { + order.push(prefix.clone()); + } + groups.entry(prefix).or_default().push(r); + } + + let mut folders: Vec<(String, Vec)> = Vec::new(); + let mut singles: Vec<(String, Vec)> = Vec::new(); + for name in order { + let items = groups.remove(&name).unwrap_or_default(); + if items.len() >= 2 { + folders.push((name, items)); + } else { + singles.push((name, items)); + } + } + if bucket_sort.column == SearchColumn::Path { + folders.sort_by(|a, b| compare_strings(&a.0, &b.0, bucket_sort.direction)); + singles.sort_by(|a, b| compare_strings(&a.0, &b.0, bucket_sort.direction)); + } + + for (prefix, items) in folders { + if folded { + rows.push(Row::FoldedGroup { + prefix, + count: items.len(), + indent: store_indent, + }); + continue; + } + let shared = Some(prefix); + for result in items { + rows.push(Row::Secret { + result, + indent: store_indent, + shared_prefix: shared.clone(), + }); + } + } + for (_, items) in singles { + for result in items { + rows.push(Row::Secret { + result, + indent: store_indent, + shared_prefix: None, + }); + } + } +} + +/// Top-level path segment of a secret's path, used for prefix grouping. +pub fn prefix_of(path: &str) -> &str { + match path.split_once('/') { + Some((head, _)) => head, + None => path, + } +} + +/// Split `parent` (the slash-terminated path prefix in front of a secret's +/// basename) into a leading "shared" segment and the remainder. The shared +/// segment is `"/"` when the leaf is part of a multi-leaf group; +/// otherwise the entire parent stays in the second slot for the dimmed +/// renderer to draw as before. +pub fn split_shared_prefix<'a>(parent: &'a str, shared: Option<&str>) -> (&'a str, &'a str) { + let Some(prefix) = shared else { + return ("", parent); + }; + let head = format!("{prefix}/"); + if parent.starts_with(&head) { + parent.split_at(head.len()) + } else { + ("", parent) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn result(path: &str, store: &str) -> SearchResult { + SearchResult { + path: path.to_string(), + store: store.to_string(), + ..Default::default() + } + } + + fn default_sort() -> SortState { + SortState { + column: SearchColumn::Path, + direction: SortDirection::Asc, + } + } + + #[test] + fn folded_mode_collapses_multi_leaf_groups_only() { + let results = vec![ + result("prod/a", "s"), + result("prod/b", "s"), + result("lonely", "s"), + ]; + let rows = build_rows(&results, true, default_sort()); + // One FoldedGroup for prod (2 leaves), one Secret for the singleton. + assert_eq!(rows.len(), 2); + match &rows[0] { + Row::FoldedGroup { prefix, count, .. } => { + assert_eq!(prefix, "prod"); + assert_eq!(*count, 2); + } + other => panic!("expected FoldedGroup, got {other:?}"), + } + assert!(matches!(&rows[1], Row::Secret { shared_prefix: None, .. })); + } + + #[test] + fn unfolded_mode_tags_shared_prefixes_inline() { + let results = vec![result("prod/a", "s"), result("prod/b", "s")]; + let rows = build_rows(&results, false, default_sort()); + assert_eq!(rows.len(), 2); + for row in &rows { + match row { + Row::Secret { shared_prefix, .. } => { + assert_eq!(shared_prefix.as_deref(), Some("prod")); + } + other => panic!("expected Secret, got {other:?}"), + } + } + } + + #[test] + fn multi_store_results_get_headers_and_indent() { + let results = vec![result("a", "s1"), result("b", "s2")]; + let rows = build_rows(&results, false, default_sort()); + assert_eq!(rows.len(), 4); + assert!(matches!(&rows[0], Row::Store { name, count: 1 } if name == "s1")); + assert!(matches!(&rows[1], Row::Secret { indent: 1, .. })); + assert!(matches!(&rows[2], Row::Store { name, count: 1 } if name == "s2")); + } + + #[test] + fn store_sort_desc_reverses_store_buckets_not_leaves() { + let results = vec![result("a", "s1"), result("b", "s2")]; + let rows = build_rows( + &results, + false, + SortState { + column: SearchColumn::Store, + direction: SortDirection::Desc, + }, + ); + assert!(matches!(&rows[0], Row::Store { name, .. } if name == "s2")); + } + + #[test] + fn split_shared_prefix_only_strips_matching_heads() { + assert_eq!(split_shared_prefix("prod/", Some("prod")), ("prod/", "")); + assert_eq!(split_shared_prefix("prod/", None), ("", "prod/")); + assert_eq!(split_shared_prefix("other/", Some("prod")), ("", "other/")); + } + + #[test] + fn prefix_of_takes_head_segment() { + assert_eq!(prefix_of("prod/db/KEY"), "prod"); + assert_eq!(prefix_of("KEY"), "KEY"); + } +} diff --git a/rust/src/tui/model/result_sort.rs b/rust/src/tui/model/result_sort.rs new file mode 100644 index 0000000..27b1b30 --- /dev/null +++ b/rust/src/tui/model/result_sort.rs @@ -0,0 +1,194 @@ +//! ResultSort — column/direction ordering over search results. +//! +//! Pure state module: comparators and sort state only, no rendering. The +//! search view consumes it directly; [`super::path_folding`] applies it +//! inside each store bucket. + +use crate::cli::search::SearchResult; + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum SearchColumn { + Path, + Updated, + Tags, + Description, + Store, +} + +impl SearchColumn { + pub fn label(self) -> &'static str { + match self { + SearchColumn::Path => "PATH", + SearchColumn::Updated => "UPDATED", + SearchColumn::Tags => "TAGS", + SearchColumn::Description => "DESCRIPTION", + SearchColumn::Store => "STORE", + } + } + + pub fn base_columns() -> &'static [SearchColumn] { + &[ + SearchColumn::Path, + SearchColumn::Updated, + SearchColumn::Tags, + SearchColumn::Description, + ] + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum SortDirection { + Asc, + Desc, +} + +impl SortDirection { + pub fn toggled(self) -> Self { + match self { + SortDirection::Asc => SortDirection::Desc, + SortDirection::Desc => SortDirection::Asc, + } + } + + pub fn marker(self) -> char { + match self { + SortDirection::Asc => '^', + SortDirection::Desc => 'v', + } + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct SortState { + pub column: SearchColumn, + pub direction: SortDirection, +} + +pub fn sort_results(results: &mut [SearchResult], sort_state: SortState) { + results.sort_by(|a, b| compare_results(a, b, sort_state)); +} + +/// Compare two results by the active column + direction, with a stable +/// `(store, path)` tiebreak so equal keys keep a deterministic order. +pub fn compare_results( + a: &SearchResult, + b: &SearchResult, + sort_state: SortState, +) -> std::cmp::Ordering { + let primary = match sort_state.column { + SearchColumn::Path => a.path.cmp(&b.path), + SearchColumn::Updated => result_timestamp(a).cmp(result_timestamp(b)), + SearchColumn::Tags => result_tags(a).cmp(result_tags(b)), + SearchColumn::Description => a + .description + .as_deref() + .unwrap_or("") + .cmp(b.description.as_deref().unwrap_or("")), + SearchColumn::Store => a.store.cmp(&b.store), + }; + let primary = match sort_state.direction { + SortDirection::Asc => primary, + SortDirection::Desc => primary.reverse(), + }; + primary + .then_with(|| a.store.cmp(&b.store)) + .then_with(|| a.path.cmp(&b.path)) +} + +pub fn compare_strings(a: &str, b: &str, direction: SortDirection) -> std::cmp::Ordering { + match direction { + SortDirection::Asc => a.cmp(b), + SortDirection::Desc => b.cmp(a), + } +} + +/// The timestamp a result sorts by: `updated_at`, falling back to +/// `created_at`, then empty (sorts first ascending). +pub fn result_timestamp(result: &SearchResult) -> &str { + result + .updated_at + .as_deref() + .or(result.created_at.as_deref()) + .unwrap_or("") +} + +/// The tag a result sorts by: its first tag, or empty. +pub fn result_tags(result: &SearchResult) -> &str { + result + .tags + .as_deref() + .and_then(|tags| tags.first()) + .map(String::as_str) + .unwrap_or("") +} + +#[cfg(test)] +mod tests { + use super::*; + + fn result(path: &str, store: &str, updated: Option<&str>, tag: Option<&str>) -> SearchResult { + SearchResult { + path: path.to_string(), + store: store.to_string(), + updated_at: updated.map(str::to_string), + tags: tag.map(|t| vec![t.to_string()]), + ..Default::default() + } + } + + #[test] + fn sorts_by_path_in_both_directions() { + let mut rs = vec![result("b", "s", None, None), result("a", "s", None, None)]; + sort_results( + &mut rs, + SortState { + column: SearchColumn::Path, + direction: SortDirection::Asc, + }, + ); + assert_eq!(rs[0].path, "a"); + sort_results( + &mut rs, + SortState { + column: SearchColumn::Path, + direction: SortDirection::Desc, + }, + ); + assert_eq!(rs[0].path, "b"); + } + + #[test] + fn updated_falls_back_to_created_and_ties_break_on_store_then_path() { + let mut a = result("x", "s2", None, None); + a.created_at = Some("2026-01-01".into()); + let b = result("x", "s1", Some("2026-01-01"), None); + // Same effective timestamp: tiebreak puts store s1 first. + let mut rs = vec![a, b]; + sort_results( + &mut rs, + SortState { + column: SearchColumn::Updated, + direction: SortDirection::Asc, + }, + ); + assert_eq!(rs[0].store, "s1"); + } + + #[test] + fn tag_sort_uses_first_tag_and_direction_flip_is_involutive() { + let dir = SortDirection::Asc; + assert_eq!(dir.toggled().toggled(), dir); + + let a = result("a", "s", None, Some("zeta")); + let b = result("b", "s", None, Some("alpha")); + let mut rs = vec![a, b]; + sort_results( + &mut rs, + SortState { + column: SearchColumn::Tags, + direction: SortDirection::Asc, + }, + ); + assert_eq!(rs[0].path, "b"); + } +} diff --git a/rust/src/tui/views/command_palette.rs b/rust/src/tui/views/command_palette.rs index 8ba9647..b6a30a0 100644 --- a/rust/src/tui/views/command_palette.rs +++ b/rust/src/tui/views/command_palette.rs @@ -96,21 +96,15 @@ impl Command { } } - /// The [`KeyAction`] this command shares with the keymap, if any. This is - /// the single link between the palette and the live keybindings: the - /// displayed shortcut is derived from whatever the user has bound to this - /// action, so the palette can never drift out of sync with the keymap. - /// Returns `None` for commands that have no direct key binding (they are - /// only reachable through the palette itself). + /// The [`KeyAction`] this command shares with the keymap, if any — + /// derived from the key registry's `palette` links, so the palette and + /// the keymap cannot drift (the registry row is the single source of + /// truth). Returns `None` for commands that have no direct key binding + /// (they are only reachable through the palette itself). pub fn key_action(&self) -> Option { - match self { - Command::NewSecret => Some(KeyAction::NewSecret), - Command::SwitchStore => Some(KeyAction::SwitchStore), - Command::Outputs => Some(KeyAction::Outputs), - Command::Help => Some(KeyAction::Help), - Command::Quit => Some(KeyAction::Quit), - _ => None, - } + KeyAction::ALL + .into_iter() + .find(|a| crate::tui::keymap::row(*a).palette == Some(*self)) } /// Human-facing shortcut string derived from the live keymap (e.g. @@ -611,15 +605,15 @@ mod tests { #[test] fn shortcut_is_derived_from_live_keymap_not_hardcoded() { - // Default keymap binds envs to Shift+E — displayed as `shift-e`. + // Default keymap binds outputs to Shift+E — displayed as `shift-e`. let default_km = KeyMap::default(); assert_eq!(Command::Outputs.shortcut(&default_km), "shift-e"); - // Remap envs to Ctrl+L and confirm the palette shortcut tracks the - // change. If shortcut() were still hardcoded this would fail — this - // is the regression guard against palette/keymap drift. + // Remap outputs to Ctrl+L and confirm the palette shortcut tracks + // the change. If shortcut() were still hardcoded this would fail — + // this is the regression guard against palette/keymap drift. let remapped = KeyMap { - envs: vec![KeyChord::single(KeyBinding::ctrl('l'))], + outputs: vec![KeyChord::single(KeyBinding::ctrl('l'))], ..KeyMap::default() }; assert_eq!(Command::Outputs.shortcut(&remapped), "ctrl-l"); diff --git a/rust/src/tui/views/help.rs b/rust/src/tui/views/help.rs index 8bbcd15..8143f12 100644 --- a/rust/src/tui/views/help.rs +++ b/rust/src/tui/views/help.rs @@ -28,17 +28,34 @@ pub enum HelpAction { Close, } -/// Modal help popup bound to a static set of `(key, description)` rows. +/// Modal help popup bound to a set of `(key, description)` rows. Views with +/// rebindable actions build their rows from the live keymap (via +/// [`crate::tui::keymap::help_rows`]) so user rebinds show up here; views +/// whose keys are fixed use [`HelpView::from_static`]. pub struct HelpView { - entries: &'static [(&'static str, &'static str)], + entries: Vec<(String, String)>, title: &'static str, } impl HelpView { - pub fn new(entries: &'static [(&'static str, &'static str)], title: &'static str) -> Self { + pub fn new(entries: Vec<(String, String)>, title: &'static str) -> Self { Self { entries, title } } + /// Convenience for views whose help rows carry no rebindable chords. + pub fn from_static( + entries: &'static [(&'static str, &'static str)], + title: &'static str, + ) -> Self { + Self::new( + entries + .iter() + .map(|(k, v)| (k.to_string(), v.to_string())) + .collect(), + title, + ) + } + pub fn on_key(&mut self, key: KeyEvent) -> HelpAction { match key.code { KeyCode::Esc | KeyCode::Char('?') => HelpAction::Close, @@ -84,7 +101,7 @@ impl HelpView { .add_modifier(Modifier::BOLD), ), Span::raw(" "), - Span::raw(*desc), + Span::raw(desc.clone()), ]); ListItem::new(line) }) @@ -119,19 +136,19 @@ mod tests { #[test] fn esc_closes_overlay() { - let mut view = HelpView::new(SAMPLE, "help"); + let mut view = HelpView::from_static(SAMPLE, "help"); assert_eq!(view.on_key(press(KeyCode::Esc)), HelpAction::Close); } #[test] fn question_mark_closes_overlay() { - let mut view = HelpView::new(SAMPLE, "help"); + let mut view = HelpView::from_static(SAMPLE, "help"); assert_eq!(view.on_key(press(KeyCode::Char('?'))), HelpAction::Close); } #[test] fn other_keys_keep_overlay_open() { - let mut view = HelpView::new(SAMPLE, "help"); + let mut view = HelpView::from_static(SAMPLE, "help"); assert_eq!(view.on_key(press(KeyCode::Char('q'))), HelpAction::None); assert_eq!(view.on_key(press(KeyCode::Down)), HelpAction::None); assert_eq!(view.on_key(press(KeyCode::Enter)), HelpAction::None); diff --git a/rust/src/tui/views/new_secret.rs b/rust/src/tui/views/new_secret.rs index 9a501e0..7983d5e 100644 --- a/rust/src/tui/views/new_secret.rs +++ b/rust/src/tui/views/new_secret.rs @@ -16,11 +16,11 @@ //! RFC 3339 timestamp. //! //! Tab / Shift-Tab move between fields with wrap-around. `Ctrl+S` or `Ctrl+W` -//! submits from any field. Submission encrypts via [`crate::crypto::age`] -//! and writes through [`crate::remote::store::write_secret`], reusing the -//! exact same code path that `himitsu set` uses. Validation leans on the -//! `pub(crate)` helpers in [`crate::cli::set`] and [`crate::cli::duration`] -//! so the TUI and CLI stay in lockstep. +//! submits from any field. Submission goes through +//! [`crate::cli::store_ops::set_secret`] — the same mutation core (and the +//! same commit/push/completions chain) that backs `himitsu set` — so the +//! TUI and CLI cannot drift. Validation leans on the `pub(crate)` helpers +//! in [`crate::cli::set`] and [`crate::cli::duration`]. //! //! On success the outer app router refreshes search; on failure the view //! surfaces the error in its status line and stays open so the user can @@ -43,7 +43,7 @@ use ratatui::Frame; use crate::cli::duration::{self, ExpiresAt}; use crate::cli::set::{validate_env_key, validate_totp}; use crate::cli::Context; -use crate::crypto::{age, secret_value, tags as tag_grammar}; +use crate::crypto::tags as tag_grammar; use crate::proto::SecretValue; use crate::remote::store; use crate::tui::keymap::{Bindings, KeyAction, KeyMap}; @@ -643,21 +643,6 @@ impl NewSecretView { let full = self.path.trim().to_string(); - let recipients = - match age::collect_recipients(&self.ctx.store, self.ctx.recipients_path.as_deref()) { - Ok(r) if !r.is_empty() => r, - Ok(_) => { - let msg = "no recipients configured for this store".to_string(); - self.status = Some(msg.clone()); - return NewSecretAction::Failed(msg); - } - Err(e) => { - let msg = format!("{e}"); - self.status = Some(msg.clone()); - return NewSecretAction::Failed(msg); - } - }; - let sv = match self.build_secret_value() { Ok(sv) => sv, Err(msg) => { @@ -665,24 +650,16 @@ impl NewSecretView { return NewSecretAction::Failed(msg); } }; - let wire = secret_value::encode(&sv); - let ciphertext = match age::encrypt(&wire, &recipients) { - Ok(ct) => ct, - Err(e) => { - let msg = format!("{e}"); - self.status = Some(msg.clone()); - return NewSecretAction::Failed(msg); - } - }; - if let Err(e) = store::write_secret(&self.ctx.store, &full, &ciphertext) { + // The mutation core owns the full chain — validation, encryption to + // the effective store's recipients, write, append-only commit+push, + // completions-cache refresh — so the TUI cannot drift from the CLI. + if let Err(e) = crate::cli::store_ops::set_secret(&self.ctx, &full, &sv) { let msg = format!("{e}"); self.status = Some(msg.clone()); return NewSecretAction::Failed(msg); } - self.ctx.commit_and_push(&format!("himitsu: set {full}")); - NewSecretAction::Created(full) } @@ -974,20 +951,23 @@ impl NewSecretView { frame.render_widget(Paragraph::new(line), area); } - pub fn help_entries() -> &'static [(&'static str, &'static str)] { - &[ + /// Help rows: rebindable form actions rendered from the LIVE keymap, + /// plus the form's fixed enter semantics and the hardwired Ctrl-C quit. + pub fn help_entries(keymap: &KeyMap) -> Vec<(String, String)> { + let mut rows = crate::tui::keymap::help_rows( + keymap, + crate::tui::keymap::Scope::NewSecretForm, + ); + rows.extend([ + ("enter (value)".to_string(), "insert newline".to_string()), ( - "tab / enter", - "next field (wraps); tab cycles into [ submit ]", + "enter (submit)".to_string(), + "save the new secret".to_string(), ), - ("shift-tab", "previous field (wraps)"), - ("enter (value)", "insert newline"), - ("enter (submit)", "save the new secret"), - ("ctrl-s / ctrl-w", "save from any field"), - ("esc", "cancel (prompts if any field has content)"), - ("ctrl-c", "quit"), - ("?", "toggle this help"), - ] + ]); + rows.push(crate::tui::keymap::help_row(keymap, KeyAction::Help)); + rows.push(("ctrl-c".into(), "quit".into())); + rows } pub fn help_title() -> &'static str { @@ -1044,6 +1024,7 @@ mod tests { key_provider: crate::config::KeyProvider::default(), project_root: None, git: std::sync::Arc::new(crate::git::CliGitAdapter), + project_config_cell: Default::default(), } } diff --git a/rust/src/tui/views/outputs.rs b/rust/src/tui/views/outputs.rs index 98c1c92..f90499e 100644 --- a/rust/src/tui/views/outputs.rs +++ b/rust/src/tui/views/outputs.rs @@ -1568,6 +1568,7 @@ fn clone_ctx(ctx: &Context) -> Context { key_provider: ctx.key_provider.clone(), project_root: ctx.project_root.clone(), git: ctx.git.clone(), + project_config_cell: ctx.project_config_cell.clone(), } } @@ -1655,6 +1656,7 @@ mod tests { key_provider: crate::config::KeyProvider::default(), project_root: None, git: std::sync::Arc::new(crate::git::CliGitAdapter), + project_config_cell: Default::default(), } } diff --git a/rust/src/tui/views/recipient_add.rs b/rust/src/tui/views/recipient_add.rs index d31e815..b2b1a95 100644 --- a/rust/src/tui/views/recipient_add.rs +++ b/rust/src/tui/views/recipient_add.rs @@ -75,7 +75,9 @@ impl RecipientAddView { Some(description) }; - match crate::cli::recipient::add_recipient(&self.ctx, &name, &age_key, description) { + // The mutation core runs the commit/push/completions chain — the + // recipient add is committed like its CLI equivalent. + match crate::cli::store_ops::recipient_add(&self.ctx, &name, &age_key, description) { Ok(()) => RecipientAddAction::Created(name), Err(e) => RecipientAddAction::Failed(format!("{e}")), } @@ -168,6 +170,7 @@ mod tests { key_provider: crate::config::KeyProvider::default(), project_root: None, git: std::sync::Arc::new(crate::git::CliGitAdapter), + project_config_cell: Default::default(), } } diff --git a/rust/src/tui/views/recipient_list.rs b/rust/src/tui/views/recipient_list.rs index 0e3fe32..00c3f05 100644 --- a/rust/src/tui/views/recipient_list.rs +++ b/rust/src/tui/views/recipient_list.rs @@ -133,7 +133,8 @@ impl RecipientListView { } fn do_remove(&mut self, name: &str) -> RecipientListAction { - match recipient::remove_recipient(&self.ctx, name) { + // The mutation core runs the commit/push/completions chain. + match crate::cli::store_ops::recipient_rm(&self.ctx, name) { Ok(()) => { self.reload(); RecipientListAction::Removed(name.to_string()) @@ -297,6 +298,7 @@ mod tests { key_provider: crate::config::KeyProvider::default(), project_root: None, git: std::sync::Arc::new(crate::git::CliGitAdapter), + project_config_cell: Default::default(), }; (tmp, ctx) } @@ -314,6 +316,7 @@ mod tests { key_provider: crate::config::KeyProvider::default(), project_root: None, git: std::sync::Arc::new(crate::git::CliGitAdapter), + project_config_cell: Default::default(), } } @@ -348,8 +351,8 @@ mod tests { #[test] fn lists_recipients_from_store() { let (_tmp, ctx) = mk_ctx(); - recipient::add_recipient(&ctx, "alice", AGE_KEY_1, Some("Alice".into())).unwrap(); - recipient::add_recipient(&ctx, "bob", AGE_KEY_2, None).unwrap(); + crate::cli::store_ops::recipient_add(&ctx, "alice", AGE_KEY_1, Some("Alice".into())).unwrap(); + crate::cli::store_ops::recipient_add(&ctx, "bob", AGE_KEY_2, None).unwrap(); let view = RecipientListView::new(&ctx); assert_eq!(view.entries.len(), 2); @@ -362,7 +365,7 @@ mod tests { fn delete_requires_confirmation_then_removes() { let km = KeyMap::default(); let (_tmp, ctx) = mk_ctx(); - recipient::add_recipient(&ctx, "alice", AGE_KEY_1, None).unwrap(); + crate::cli::store_ops::recipient_add(&ctx, "alice", AGE_KEY_1, None).unwrap(); let mut view = RecipientListView::new(&ctx); assert_eq!(view.entries.len(), 1); @@ -388,7 +391,7 @@ mod tests { fn delete_confirmation_cancels_on_other_key() { let km = KeyMap::default(); let (_tmp, ctx) = mk_ctx(); - recipient::add_recipient(&ctx, "alice", AGE_KEY_1, None).unwrap(); + crate::cli::store_ops::recipient_add(&ctx, "alice", AGE_KEY_1, None).unwrap(); let mut view = RecipientListView::new(&ctx); view.on_key(press(KeyCode::Char('d')), &km); diff --git a/rust/src/tui/views/remote_add.rs b/rust/src/tui/views/remote_add.rs index 14e5f21..a68c408 100644 --- a/rust/src/tui/views/remote_add.rs +++ b/rust/src/tui/views/remote_add.rs @@ -163,6 +163,7 @@ mod tests { key_provider: crate::config::KeyProvider::default(), project_root: None, git: std::sync::Arc::new(crate::git::CliGitAdapter), + project_config_cell: Default::default(), } } diff --git a/rust/src/tui/views/search.rs b/rust/src/tui/views/search.rs index 18413e7..ee35e0a 100644 --- a/rust/src/tui/views/search.rs +++ b/rust/src/tui/views/search.rs @@ -29,8 +29,10 @@ use crate::cli::search::{humanize_age_compact, parse_ts, search_core, SearchResu use crate::cli::Context; use crate::crypto::{age, secret_value}; use crate::remote::store; -use crate::tui::icons; use crate::tui::keymap::{KeyAction, KeyMap}; +use crate::tui::model::path_folding::{build_rows, prefix_of, split_shared_prefix, Row}; +use crate::tui::model::result_sort::{SearchColumn, SortDirection, SortState}; +use crate::tui::widgets::store_health::{check_store_health_pair, render_health_pill, StoreHealth}; use crate::tui::views::command_palette::{Command, CommandPalette, CommandPaletteOutcome}; use crate::tui::views::store_picker::{StorePicker, StorePickerOutcome}; use crate::tui::widgets::secret_ref_autocomplete::SecretRefAutocomplete; @@ -81,126 +83,9 @@ pub enum SearchAction { CommandHint(String), } -/// A row in the rendered results list. `Store` headers group secrets by -/// origin (`org/repo` slug or local path) and are never selectable; -/// navigation steps over them. `FoldedGroup` rows appear only in folded mode, -/// one per top-level path prefix shared by ≥ 2 secrets — they collapse the -/// group's leaves into a single selectable row that expands when the user -/// unfolds. -#[derive(Debug, Clone)] -enum Row { - Store { - name: String, - count: usize, - }, - FoldedGroup { - /// Top-level path segment shared by the collapsed leaves. - prefix: String, - /// Number of leaves under this prefix. - count: usize, - /// Indentation depth (matches what its children would have if - /// expanded). 0 in single-store mode, 1 under a `Store` header. - indent: usize, - }, - Secret { - result: SearchResult, - /// Indentation depth in list-item cells (2 spaces per level). - /// 0 in single-store mode, 1 under a `Store` header. - indent: usize, - /// Top-level path segment when this secret shares a prefix with - /// ≥ 1 sibling in the same store. The renderer paints this segment - /// with a subtle accent so the visual grouping survives without a - /// separate header row. `None` for singletons. - shared_prefix: Option, - }, -} - -/// Health status of the active store's git checkout, computed once at view -/// construction. Displayed as a compact indicator in the header bar. -#[derive(Debug, Clone)] -enum StoreHealth { - /// Store checkout is up to date with its remote tracking branch. - Synced, - /// Local checkout is behind its remote by N commit(s). - Behind(u32), - /// Working tree has uncommitted local changes. - Dirty, - /// Both behind remote AND has local changes. - BehindAndDirty(u32), - /// Store directory is not a git repo. - NotGit, - /// Git repo exists but has no remote configured. - NoRemote, - /// Git repo has a remote but the tracking branch doesn't exist yet - /// (e.g. never pushed). - NotPushed, - /// User's own age key is not in the store's recipient list. - NotRecipient, - /// Could not determine status for some other reason. - Unknown, -} - -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -enum SearchColumn { - Path, - Updated, - Tags, - Description, - Store, -} - const SEARCH_COLUMN_MAX_WIDTH: usize = 32; const TRUNCATION_MARKER: &str = ".."; -impl SearchColumn { - fn label(self) -> &'static str { - match self { - SearchColumn::Path => "PATH", - SearchColumn::Updated => "UPDATED", - SearchColumn::Tags => "TAGS", - SearchColumn::Description => "DESCRIPTION", - SearchColumn::Store => "STORE", - } - } - - fn base_columns() -> &'static [SearchColumn] { - &[ - SearchColumn::Path, - SearchColumn::Updated, - SearchColumn::Tags, - SearchColumn::Description, - ] - } -} - -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -enum SortDirection { - Asc, - Desc, -} - -impl SortDirection { - fn toggled(self) -> Self { - match self { - SortDirection::Asc => SortDirection::Desc, - SortDirection::Desc => SortDirection::Asc, - } - } - - fn marker(self) -> char { - match self { - SortDirection::Asc => '^', - SortDirection::Desc => 'v', - } - } -} - -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -struct SortState { - column: SearchColumn, - direction: SortDirection, -} - pub struct SearchView { query: String, results: Vec, @@ -269,6 +154,7 @@ impl SearchView { key_provider: ctx.key_provider.clone(), project_root: ctx.project_root.clone(), git: ctx.git.clone(), + project_config_cell: ctx.project_config_cell.clone(), }; let (global_health, project_health) = check_store_health_pair(&ctx_owned); let mut view = Self { @@ -337,24 +223,10 @@ impl SearchView { } } - // Ctrl+Space opens the autocomplete popup. We re-toggle it (rather - // than only opening) so a user who pulled it up by accident can - // dismiss it with the same chord. - if matches!(key.code, KeyCode::Char(' ')) && key.modifiers.contains(KeyModifiers::CONTROL) { - let want_open = !self.autocomplete.is_open(); - self.autocomplete.set_open(want_open); - return SearchAction::None; - } - if matches!(key.code, KeyCode::Char('t')) && key.modifiers.contains(KeyModifiers::CONTROL) { - return self.refine_to_selected_tag(); - } - if matches!(key.code, KeyCode::Char('o')) && key.modifiers.contains(KeyModifiers::CONTROL) { - self.sort_by_selected_column(); - return SearchAction::None; - } - // Ctrl+- / Ctrl++ (collapse/expand paths) are keymap-driven actions - // routed through `dispatch_action` above (and reachable via the leader - // chord `Ctrl+x -` / `Ctrl+x +`), so no hardcoded handling here. + // Autocomplete toggle, tag refine, and column sort are keymap-driven + // actions (ToggleAutocomplete / RefineTag / SortColumn) routed + // through `dispatch_action` above — rebindable like everything else, + // no hardcoded chords here. // Esc closes the popup before falling through to the view's own // cancel/quit semantics. if key.code == KeyCode::Esc && self.autocomplete.is_open() { @@ -463,6 +335,18 @@ impl SearchView { self.set_folded(false); Some(SearchAction::None) } + KeyAction::ToggleAutocomplete => { + // Re-toggle (rather than only open) so a user who pulled the + // popup up by accident can dismiss it with the same chord. + let want_open = !self.autocomplete.is_open(); + self.autocomplete.set_open(want_open); + Some(SearchAction::None) + } + KeyAction::RefineTag => Some(self.refine_to_selected_tag()), + KeyAction::SortColumn => { + self.sort_by_selected_column(); + Some(SearchAction::None) + } _ => None, } } @@ -817,12 +701,14 @@ impl SearchView { } fn run_sync(&mut self) -> SearchAction { - use crate::cli::rekey; + use crate::cli::store_ops; if let Err(e) = crate::git::pull(&self.ctx.store) { return SearchAction::CommandFailed(format!("sync pull failed: {e}")); } - match rekey::rekey_store(&self.ctx, None) { + // The mutation core owns the commit/push/completions chain — the + // rekeyed store is never left with a dirty tree. + match store_ops::rekey(&self.ctx, None) { Ok(n) => { self.refresh_results(); let (g, p) = check_store_health_pair(&self.ctx); @@ -835,8 +721,8 @@ impl SearchView { } fn run_rekey(&self) -> SearchAction { - use crate::cli::rekey; - match rekey::rekey_store(&self.ctx, None) { + use crate::cli::store_ops; + match store_ops::rekey(&self.ctx, None) { Ok(n) => { let recipients = crate::crypto::age::collect_recipients( &self.ctx.store, @@ -853,21 +739,14 @@ impl SearchView { } fn run_join(&mut self) -> SearchAction { - use crate::cli::join::{self, JoinArgs}; - - if join::is_self_recipient(&self.ctx) { - return SearchAction::Joined("already a recipient".into()); - } - - match join::run( - JoinArgs { - name: None, - no_push: false, - }, - &self.ctx, - ) { - Ok(()) => { - self.ctx.commit_and_push("himitsu: join"); + use crate::cli::join::JoinOutcome; + use crate::cli::store_ops; + + // The silent core is idempotent and never prints (printing would + // corrupt ratatui); the chain commits and pushes on success. + match store_ops::join(&self.ctx) { + Ok(JoinOutcome::AlreadyRecipient) => SearchAction::Joined("already a recipient".into()), + Ok(JoinOutcome::Joined(_)) => { let (g, p) = check_store_health_pair(&self.ctx); self.global_health = g; self.project_health = p; @@ -1419,6 +1298,9 @@ const SEARCH_ACTION_PRIORITY: &[KeyAction] = &[ KeyAction::CopySelected, KeyAction::CollapsePaths, KeyAction::ExpandPaths, + KeyAction::ToggleAutocomplete, + KeyAction::RefineTag, + KeyAction::SortColumn, ]; fn match_keymap_action(keymap: &KeyMap, key: &crossterm::event::KeyEvent) -> Option { @@ -1458,172 +1340,10 @@ fn decrypt_value(ctx: &Context, result: &SearchResult) -> crate::error::Result (StoreHealth, Option) { - let global_health = check_store_health(ctx); - - let project_health = match resolve_project_store(ctx) { - Some(project_store) => { - let mut project_ctx = ctx.clone(); - project_ctx.store = project_store; - Some(check_store_health(&project_ctx)) - } - None => None, - }; - (global_health, project_health) -} - -/// Find the project store referenced by the current repo's `himitsu.yaml`, -/// if any. Returns `None` when there's no project config, no `default_store` -/// in it, or the slug doesn't resolve to an existing checkout under -/// `stores_dir`. -fn resolve_project_store(ctx: &Context) -> Option { - let (project_cfg, _) = crate::config::load_project_config().ok()??; - let slug = project_cfg.default_store?; - let (org, repo) = crate::config::validate_remote_slug(&slug).ok()?; - let candidate = ctx.stores_dir().join(org).join(repo); - candidate.exists().then_some(candidate) -} - -/// Render a labelled health pill: `