secretenv_backend_1password/

lib.rs

// Copyright (C) 2026 Mandeep Patel
// SPDX-License-Identifier: AGPL-3.0-only

//! 1Password backend for SecretEnv.
//!
//! Wraps the `op` CLI — never 1Password's Connect Server SDK. The CLI
//! handles SSO, biometric unlock, service accounts, and multiple signed-
//! in accounts without us touching any of it.
//!
//! # URI shape
//!
//! `<instance>://<vault>/<item>/<field>` — exactly three non-empty path
//! segments. Example: `1password-personal://Engineering/Prod DB/url`
//! targets the `url` field of the `Prod DB` item in the `Engineering`
//! vault.
//!
//! Nested fields (items with sections, `op://vault/item/section/field`)
//! are out of scope for v0.1; the strict 3-segment rule is documented
//! in the error message when parsing fails.
//!
//! # Config fields
//!
//! - `op_account` (optional) — 1Password account shorthand or URL
//!   (e.g. `myteam.1password.com`). Needed only when multiple accounts
//!   are signed in simultaneously. Passed as `--account <value>` to
//!   every `op` invocation.
//!
//! # Semantics
//!
//! - [`get`](OnePasswordBackend) calls `op read op://<v>/<i>/<f>` and
//!   returns the field value verbatim.
//! - [`set`](OnePasswordBackend) calls `op item edit <item> <field>=<value> --vault <vault>`.
//!   Errors if the item does not exist — we never auto-create.
//! - [`delete`](OnePasswordBackend) calls `op item edit <item> <field>= --vault <vault>`
//!   (empty value). Full item deletion is out of scope for v0.1.
//! - [`list`](OnePasswordBackend) fetches the field value and parses it
//!   as flat TOML `HashMap<String, String>`. This is the registry-
//!   document shape: a 1Password note whose body is the alias → URI
//!   map in TOML form.
//! - [`check`](OnePasswordBackend) runs `op --version` (Level 1) and
//!   `op whoami --format=json` (Level 2).
//!
//! # Safety
//!
//! Every argv call goes through `tokio::process::Command::args(&[…])`
//! — never `sh -c`. URI-derived values never touch a shell interpreter.
#![forbid(unsafe_code)]
#![allow(clippy::module_name_repetitions)]

use std::collections::HashMap;
use std::io;
use std::time::Duration;

use anyhow::{anyhow, bail, Context, Result};
use async_trait::async_trait;
use secretenv_core::{
    optional_bool, optional_duration_secs, Backend, BackendFactory, BackendStatus, BackendUri,
    Secret, DEFAULT_GET_TIMEOUT,
};
use serde::Deserialize;
use tokio::process::Command;

const CLI_NAME: &str = "op";
const INSTALL_HINT: &str =
    "install via https://developer.1password.com/docs/cli/get-started/  (Homebrew: brew install 1password-cli)";

/// A live instance of the 1Password backend.
pub struct OnePasswordBackend {
    backend_type: &'static str,
    instance_name: String,
    op_account: Option<String>,
    /// Path or name of the `op` binary. Defaults to `"op"` (PATH
    /// lookup); tests override to a mock script path.
    op_bin: String,
    /// Per-instance fetch deadline; from `timeout_secs` config field.
    timeout: Duration,
    /// **Opt-in to the v0.3 CV-1 limitation.** Default `false` —
    /// `set` errors out with a clean message rather than silently
    /// passing the secret value through child argv. Set
    /// `op_unsafe_set = true` in `[backends.<name>]` to acknowledge
    /// the exposure (`/proc/<pid>/cmdline` on multi-user Linux
    /// hosts) and proceed with the legacy argv-based behavior. The
    /// `op` CLI offers no portable stdin-fed value form across the
    /// 1.x and 2.x generations, so the safer default is to refuse.
    op_unsafe_set: bool,
}

#[derive(Deserialize)]
struct WhoAmI {
    url: String,
    #[serde(default)]
    email: String,
}

impl OnePasswordBackend {
    /// Parse `uri.path` into `(vault, item, field)`. Exactly 3 non-empty
    /// `/`-separated segments; a leading `/` is tolerated.
    fn parse_path(uri: &BackendUri) -> Result<(String, String, String)> {
        let path = uri.path.strip_prefix('/').unwrap_or(&uri.path);
        let parts: Vec<&str> = path.split('/').collect();
        if parts.len() != 3 || parts.iter().any(|s| s.is_empty()) {
            bail!(
                "1password URI '{}' must have exactly three non-empty path segments \
                 (vault/item/field); got {} — v0.1 does not support nested sections",
                uri.raw,
                parts.len()
            );
        }
        Ok((parts[0].to_owned(), parts[1].to_owned(), parts[2].to_owned()))
    }

    fn append_account(&self, cmd: &mut Command) {
        if let Some(account) = &self.op_account {
            cmd.args(["--account", account]);
        }
    }

    fn cli_missing() -> BackendStatus {
        BackendStatus::CliMissing {
            cli_name: CLI_NAME.to_owned(),
            install_hint: INSTALL_HINT.to_owned(),
        }
    }

    fn operation_failure_message(&self, uri: &BackendUri, op: &str, stderr: &[u8]) -> String {
        let stderr_str = String::from_utf8_lossy(stderr).trim().to_owned();
        format!(
            "1password backend '{}': {op} failed for URI '{}': {stderr_str}",
            self.instance_name, uri.raw
        )
    }
}

#[async_trait]
impl Backend for OnePasswordBackend {
    fn backend_type(&self) -> &str {
        self.backend_type
    }

    fn instance_name(&self) -> &str {
        &self.instance_name
    }

    fn timeout(&self) -> Duration {
        self.timeout
    }

    #[allow(clippy::similar_names)]
    async fn check(&self) -> BackendStatus {
        // v0.3: Level 1 + Level 2 via `tokio::join!`. Halves `doctor`
        // latency for this backend.
        let version_fut = Command::new(&self.op_bin).arg("--version").output();
        let mut whoami_cmd = Command::new(&self.op_bin);
        whoami_cmd.args(["whoami", "--format=json"]);
        self.append_account(&mut whoami_cmd);
        let whoami_fut = whoami_cmd.output();
        let (version_res, whoami_res) = tokio::join!(version_fut, whoami_fut);

        // --- Level 1 ---
        let version_out = match version_res {
            Ok(o) => o,
            Err(e) if e.kind() == io::ErrorKind::NotFound => return Self::cli_missing(),
            Err(e) => {
                return BackendStatus::Error {
                    message: format!(
                        "1password backend '{}': failed to invoke '{}': {e}",
                        self.instance_name, self.op_bin
                    ),
                };
            }
        };
        if !version_out.status.success() {
            return BackendStatus::Error {
                message: format!(
                    "1password backend '{}': 'op --version' exited non-zero: {}",
                    self.instance_name,
                    String::from_utf8_lossy(&version_out.stderr).trim()
                ),
            };
        }
        let version_str = String::from_utf8_lossy(&version_out.stdout).trim().to_owned();
        let cli_version = format!("op/{version_str}");

        // --- Level 2 ---
        let whoami_out = match whoami_res {
            Ok(o) => o,
            Err(e) => {
                return BackendStatus::Error {
                    message: format!(
                        "1password backend '{}': failed to invoke whoami: {e}",
                        self.instance_name
                    ),
                };
            }
        };
        if !whoami_out.status.success() {
            let stderr = String::from_utf8_lossy(&whoami_out.stderr).trim().to_owned();
            let signin_hint = self
                .op_account
                .as_ref()
                .map_or_else(|| "op signin".to_owned(), |a| format!("op signin --account {a}"));
            return BackendStatus::NotAuthenticated {
                hint: format!("run: {signin_hint}  (stderr: {stderr})"),
            };
        }
        let who: WhoAmI = match serde_json::from_slice(&whoami_out.stdout) {
            Ok(w) => w,
            Err(e) => {
                return BackendStatus::Error {
                    message: format!(
                        "1password backend '{}': parsing whoami JSON: {e}",
                        self.instance_name
                    ),
                };
            }
        };
        let email_part =
            if who.email.is_empty() { String::new() } else { format!(" email={}", who.email) };
        BackendStatus::Ok { cli_version, identity: format!("account={}{email_part}", who.url) }
    }

    // `check_extensive` uses the `Backend` trait default (list().len()).

    async fn get(&self, uri: &BackendUri) -> Result<Secret<String>> {
        uri.reject_any_fragment("1password")?;
        let (vault, item, field) = Self::parse_path(uri)?;
        // Defensive post-condition: parse_path only returns 3 segments
        // each free of `/` — `op://<vault>/<item>/<field>` construction
        // is safe. Assert to guard against a future parse_path refactor.
        debug_assert!(!vault.contains('/') && !item.contains('/') && !field.contains('/'));
        let op_uri = format!("op://{vault}/{item}/{field}");
        let mut cmd = Command::new(&self.op_bin);
        cmd.args(["read", &op_uri]);
        self.append_account(&mut cmd);
        let output = cmd.output().await.with_context(|| {
            format!(
                "1password backend '{}': failed to invoke 'op read' for URI '{}'",
                self.instance_name, uri.raw
            )
        })?;
        if !output.status.success() {
            bail!(self.operation_failure_message(uri, "get", &output.stderr));
        }
        let stdout = String::from_utf8(output.stdout).with_context(|| {
            format!(
                "1password backend '{}': non-UTF-8 response for URI '{}'",
                self.instance_name, uri.raw
            )
        })?;
        Ok(Secret::new(stdout.strip_suffix("\n").unwrap_or(&stdout).to_owned()))
    }

    async fn set(&self, uri: &BackendUri, value: &str) -> Result<()> {
        uri.reject_any_fragment("1password")?;
        // KNOWN LIMITATION (review finding CV-1): the 1Password CLI
        // (`op item edit`) reads field assignments as `field=value`
        // argv tokens. Across the 1.x and 2.x CLI generations users
        // have installed, `op` offers no portable stdin-fed value
        // form (the closest, `op item create --template -`, doesn't
        // apply to in-place field edits). Until that lands upstream,
        // secretenv refuses the operation by default.
        //
        // Exposure: the secret value is visible in the child
        // process's argv for the lifetime of the `op` subprocess
        // (normally O(200 ms) — 2 s). On multi-user Linux hosts,
        // `/proc/<pid>/cmdline` is world-readable. Operators who
        // accept this exposure (single-user host, audited shared
        // host) opt in by setting `op_unsafe_set = true` under
        // `[backends.<instance>]` in config.toml.
        if !self.op_unsafe_set {
            bail!(
                "1password backend '{instance}': refusing `set` for URI '{uri}' — the `op` CLI \
                 has no portable stdin-fed value form across 1.x/2.x, so the secret would pass \
                 through subprocess argv (visible via /proc/<pid>/cmdline on multi-user Linux \
                 hosts). Either edit the field manually with `op item edit`, or opt in by adding \
                 `op_unsafe_set = true` to [backends.{instance}] (acknowledging the exposure on \
                 single-user / audited hosts).",
                instance = self.instance_name,
                uri = uri.raw,
            );
        }
        let (vault, item, field) = Self::parse_path(uri)?;
        let assignment = format!("{field}={value}");
        tracing::warn!(
            instance = self.instance_name.as_str(),
            uri = uri.raw.as_str(),
            "`op item edit` passes the field value through subprocess argv \
             (op_unsafe_set = true was set; CV-1 exposure acknowledged) — \
             do not run on multi-user hosts unless audited"
        );
        let mut cmd = Command::new(&self.op_bin);
        cmd.args(["item", "edit", &item, &assignment, "--vault", &vault]);
        self.append_account(&mut cmd);
        let output = cmd.output().await.with_context(|| {
            format!(
                "1password backend '{}': failed to invoke 'op item edit' for URI '{}'",
                self.instance_name, uri.raw
            )
        })?;
        if !output.status.success() {
            bail!(self.operation_failure_message(uri, "set", &output.stderr));
        }
        Ok(())
    }

    /// v0.15 migrate destination path. `Gated` per the v0.15 audit
    /// table — refuses unless `op_unsafe_set = true` in
    /// `[backends.<instance>]`. Returns a typed
    /// [`BackendError::WriteNotSupported`](secretenv_core::BackendError::WriteNotSupported)
    /// so the migrate handler can dispatch on the variant rather
    /// than parse the underlying `set()` error's context string.
    async fn write_secret(&self, uri: &BackendUri, value: &Secret<String>) -> Result<()> {
        if !self.op_unsafe_set {
            return Err(secretenv_core::BackendError::WriteNotSupported {
                backend_type: self.backend_type().to_owned(),
                reason: "op_unsafe_set is false — set the flag in [backends.<instance>] of config.toml to opt in",
            }
            .into());
        }
        self.set(uri, value.expose_secret()).await
    }

    /// v0.15 migrate `--delete-source` cleanup path. `Gated` per the
    /// v0.15 audit table — refuses unless `op_unsafe_set = true`,
    /// same gate as `write_secret`. Wraps `delete()` once authorized.
    async fn delete_secret(&self, uri: &BackendUri) -> Result<()> {
        if !self.op_unsafe_set {
            return Err(secretenv_core::BackendError::DeleteNotSupported {
                backend_type: self.backend_type().to_owned(),
                reason: "op_unsafe_set is false — set the flag in [backends.<instance>] of config.toml to opt in",
            }
            .into());
        }
        self.delete(uri).await
    }

    /// v0.15 migrate success-message cleanup hint. Aligned with this
    /// backend's `delete()` semantics: 1Password CLI has no field-
    /// removal — `delete()` clears the field by setting it to the
    /// empty string. The hint mirrors that exact operation rather
    /// than `op item delete <item>` (whole-item) so a copy-paste does
    /// not remove adjacent fields the operator did not intend to
    /// touch. Phase 7 audit fix — code-rev S2.
    fn delete_hint(&self, uri: &BackendUri) -> String {
        let account_flag =
            self.op_account.as_deref().map_or_else(String::new, |a| format!(" --account {a}"));
        if let Ok((vault, item, field)) = Self::parse_path(uri) {
            format!(
                "# 1Password has no field-removal; this clears the field to empty:\n\
                 op item edit \"{item}\" \"{field}=\" --vault \"{vault}\"{account_flag}"
            )
        } else {
            format!(
                "# 1Password has no field-removal; clear the field by setting it empty:\n\
                 op item edit \"<item>\" \"<field>=\" --vault \"<vault>\"{account_flag}"
            )
        }
    }

    async fn delete(&self, uri: &BackendUri) -> Result<()> {
        uri.reject_any_fragment("1password")?;
        let (vault, item, field) = Self::parse_path(uri)?;
        // 1Password's CLI has no "clear one field" — we set it to empty.
        // Full item deletion is out of scope for v0.1.
        let assignment = format!("{field}=");
        let mut cmd = Command::new(&self.op_bin);
        cmd.args(["item", "edit", &item, &assignment, "--vault", &vault]);
        self.append_account(&mut cmd);
        let output = cmd.output().await.with_context(|| {
            format!(
                "1password backend '{}': failed to invoke 'op item edit' for URI '{}'",
                self.instance_name, uri.raw
            )
        })?;
        if !output.status.success() {
            bail!(self.operation_failure_message(uri, "delete", &output.stderr));
        }
        Ok(())
    }

    async fn list(&self, uri: &BackendUri) -> Result<Vec<(String, String)>> {
        let body = self.get(uri).await?;
        let parsed: HashMap<String, String> =
            toml::from_str(body.expose_secret()).with_context(|| {
                format!(
                    "1password backend '{}': field at '{}' is not a flat TOML key→string map \
                 (v0.1 only supports registry-document shape)",
                    self.instance_name, uri.raw
                )
            })?;
        Ok(parsed.into_iter().collect())
    }

    fn registry_format(&self) -> secretenv_core::RegistryFormat {
        secretenv_core::RegistryFormat::Toml
    }
}

/// Factory for the 1Password backend. No required config fields;
/// `op_account` is optional.
pub struct OnePasswordFactory(&'static str);

impl OnePasswordFactory {
    /// Construct the factory. Equivalent to `OnePasswordFactory::default()`.
    #[must_use]
    pub const fn new() -> Self {
        Self("1password")
    }
}

impl Default for OnePasswordFactory {
    fn default() -> Self {
        Self::new()
    }
}

impl BackendFactory for OnePasswordFactory {
    fn backend_type(&self) -> &str {
        self.0
    }

    fn create(
        &self,
        instance_name: &str,
        config: &HashMap<String, toml::Value>,
    ) -> Result<Box<dyn Backend>> {
        let op_account = match config.get("op_account") {
            None => None,
            Some(v) => Some(v.as_str().map(str::to_owned).ok_or_else(|| {
                anyhow!(
                    "1password instance '{instance_name}': field 'op_account' must be a \
                     string, got {}",
                    v.type_str()
                )
            })?),
        };
        let timeout = optional_duration_secs(config, "timeout_secs", "1password", instance_name)?
            .unwrap_or(DEFAULT_GET_TIMEOUT);
        let op_unsafe_set =
            optional_bool(config, "op_unsafe_set", "1password", instance_name)?.unwrap_or(false);
        Ok(Box::new(OnePasswordBackend {
            backend_type: "1password",
            instance_name: instance_name.to_owned(),
            op_account,
            op_bin: CLI_NAME.to_owned(),
            timeout,
            op_unsafe_set,
        }))
    }
}

#[cfg(test)]
#[allow(clippy::unwrap_used, clippy::expect_used)]
mod tests {
    use std::path::Path;

    use secretenv_testing::{Response, StrictMock};
    use tempfile::TempDir;

    use super::*;

    fn backend(mock_path: &Path, account: Option<&str>) -> OnePasswordBackend {
        // Default test backend opts INTO unsafe-set so the existing
        // strict-mock argv lock for `set` keeps working. The
        // dedicated `set_refuses_when_op_unsafe_set_is_false` test
        // covers the new default-off behavior.
        OnePasswordBackend {
            backend_type: "1password",
            instance_name: "1password-personal".to_owned(),
            op_account: account.map(ToOwned::to_owned),
            op_bin: mock_path.to_str().unwrap().to_owned(),
            timeout: DEFAULT_GET_TIMEOUT,
            op_unsafe_set: true,
        }
    }

    fn backend_safe_default(mock_path: &Path) -> OnePasswordBackend {
        OnePasswordBackend {
            backend_type: "1password",
            instance_name: "1password-personal".to_owned(),
            op_account: None,
            op_bin: mock_path.to_str().unwrap().to_owned(),
            timeout: DEFAULT_GET_TIMEOUT,
            op_unsafe_set: false,
        }
    }

    fn backend_with_nonexistent_op() -> OnePasswordBackend {
        OnePasswordBackend {
            backend_type: "1password",
            instance_name: "1password-personal".to_owned(),
            op_account: None,
            op_bin: "/definitely/not/a/real/path/to/op-binary-98765".to_owned(),
            timeout: DEFAULT_GET_TIMEOUT,
            op_unsafe_set: false,
        }
    }

    // ---- Factory ----

    #[test]
    fn factory_builds_backend_with_no_required_fields() {
        let factory = OnePasswordFactory::new();
        let cfg: HashMap<String, toml::Value> = HashMap::new();
        let b = factory.create("1password-personal", &cfg).unwrap();
        assert_eq!(b.backend_type(), "1password");
        assert_eq!(b.instance_name(), "1password-personal");
    }

    #[test]
    fn factory_accepts_op_account() {
        let factory = OnePasswordFactory::new();
        let mut cfg: HashMap<String, toml::Value> = HashMap::new();
        cfg.insert("op_account".to_owned(), toml::Value::String("myteam.1password.com".to_owned()));
        assert!(factory.create("1password-team", &cfg).is_ok());
    }

    #[test]
    fn factory_rejects_non_string_op_account() {
        let factory = OnePasswordFactory::new();
        let mut cfg: HashMap<String, toml::Value> = HashMap::new();
        cfg.insert("op_account".to_owned(), toml::Value::Boolean(true));
        let Err(err) = factory.create("1password-team", &cfg) else {
            panic!("expected error for non-string op_account");
        };
        let msg = format!("{err:#}");
        assert!(msg.contains("op_account"), "names the field: {msg}");
    }

    #[test]
    fn factory_backend_type_is_1password() {
        assert_eq!(OnePasswordFactory::new().backend_type(), "1password");
    }

    #[test]
    fn factory_op_unsafe_set_accepts_true() {
        let factory = OnePasswordFactory::new();
        let mut cfg: HashMap<String, toml::Value> = HashMap::new();
        cfg.insert("op_unsafe_set".to_owned(), toml::Value::Boolean(true));
        assert!(factory.create("1password-personal", &cfg).is_ok());
    }

    #[test]
    fn factory_rejects_non_bool_op_unsafe_set() {
        let factory = OnePasswordFactory::new();
        let mut cfg: HashMap<String, toml::Value> = HashMap::new();
        cfg.insert("op_unsafe_set".to_owned(), toml::Value::String("yes".to_owned()));
        let Err(err) = factory.create("1password-personal", &cfg) else {
            panic!("expected error for non-bool op_unsafe_set");
        };
        let msg = format!("{err:#}");
        assert!(msg.contains("op_unsafe_set"), "names the field: {msg}");
        assert!(msg.contains("must be a boolean"), "describes type mismatch: {msg}");
    }

    #[test]
    fn factory_honors_timeout_secs() {
        let factory = OnePasswordFactory::new();
        let mut cfg: HashMap<String, toml::Value> = HashMap::new();
        cfg.insert("timeout_secs".to_owned(), toml::Value::Integer(17));
        let b = factory.create("1password-personal", &cfg).unwrap();
        assert_eq!(b.timeout(), Duration::from_secs(17));
    }

    #[test]
    fn factory_uses_default_timeout_when_omitted() {
        let factory = OnePasswordFactory::new();
        let cfg: HashMap<String, toml::Value> = HashMap::new();
        let b = factory.create("1password-personal", &cfg).unwrap();
        assert_eq!(b.timeout(), DEFAULT_GET_TIMEOUT);
    }

    #[test]
    fn factory_rejects_non_integer_timeout_secs() {
        let factory = OnePasswordFactory::new();
        let mut cfg: HashMap<String, toml::Value> = HashMap::new();
        cfg.insert("timeout_secs".to_owned(), toml::Value::String("30".to_owned()));
        let Err(err) = factory.create("1password-personal", &cfg) else {
            panic!("expected error for non-integer timeout_secs");
        };
        let msg = format!("{err:#}");
        assert!(msg.contains("timeout_secs"), "names the field: {msg}");
        assert!(msg.contains("must be an integer"), "describes type mismatch: {msg}");
    }

    // ---- URI parsing ----

    #[test]
    fn parse_path_three_segments_happy() {
        let uri = BackendUri::parse("1password-personal://Engineering/Prod DB/url").unwrap();
        let (v, i, f) = OnePasswordBackend::parse_path(&uri).unwrap();
        assert_eq!(v, "Engineering");
        assert_eq!(i, "Prod DB");
        assert_eq!(f, "url");
    }

    #[test]
    fn parse_path_tolerates_leading_slash() {
        let uri = BackendUri::parse("1password-personal:///Engineering/DB/url").unwrap();
        let (v, i, f) = OnePasswordBackend::parse_path(&uri).unwrap();
        assert_eq!(v, "Engineering");
        assert_eq!(i, "DB");
        assert_eq!(f, "url");
    }

    #[test]
    fn parse_path_rejects_two_segments() {
        let uri = BackendUri::parse("1password-personal://vault/item").unwrap();
        let err = OnePasswordBackend::parse_path(&uri).unwrap_err();
        assert!(format!("{err:#}").contains("vault/item/field"));
    }

    #[test]
    fn parse_path_rejects_four_segments() {
        let uri = BackendUri::parse("1password-personal://vault/item/section/field").unwrap();
        let err = OnePasswordBackend::parse_path(&uri).unwrap_err();
        assert!(format!("{err:#}").contains("three"));
    }

    #[test]
    fn parse_path_rejects_empty_segment() {
        let uri = BackendUri::parse("1password-personal://vault//field").unwrap();
        let err = OnePasswordBackend::parse_path(&uri).unwrap_err();
        assert!(format!("{err:#}").contains("non-empty"));
    }

    // ---- get happy path ----

    #[tokio::test]
    async fn get_returns_field_value_no_account() {
        let dir = TempDir::new().unwrap();
        let mock = StrictMock::new("op")
            .on(&["read", "op://Eng/API/key"], Response::success("super-secret-value\n"))
            .install(dir.path());
        let b = backend(&mock, None);
        let uri = BackendUri::parse("1password-personal://Eng/API/key").unwrap();
        assert_eq!(b.get(&uri).await.unwrap().expose_secret(), "super-secret-value");
    }

    #[tokio::test]
    async fn get_returns_field_value_with_account() {
        let dir = TempDir::new().unwrap();
        let mock = StrictMock::new("op")
            .on(
                &["read", "op://Eng/API/key", "--account", "myteam.1password.com"],
                Response::success("value-from-team\n"),
            )
            .install(dir.path());
        let b = backend(&mock, Some("myteam.1password.com"));
        let uri = BackendUri::parse("1password-team://Eng/API/key").unwrap();
        assert_eq!(b.get(&uri).await.unwrap().expose_secret(), "value-from-team");
    }

    #[tokio::test]
    async fn get_strips_single_trailing_newline() {
        let dir = TempDir::new().unwrap();
        let mock = StrictMock::new("op")
            .on(&["read", "op://V/I/F"], Response::success("raw-secret\n"))
            .install(dir.path());
        let b = backend(&mock, None);
        let uri = BackendUri::parse("1password-personal://V/I/F").unwrap();
        assert_eq!(b.get(&uri).await.unwrap().expose_secret(), "raw-secret");
    }

    // ---- get errors ----

    #[tokio::test]
    async fn get_item_not_found_error_includes_stderr_and_uri() {
        let dir = TempDir::new().unwrap();
        let mock = StrictMock::new("op")
            .on(
                &["read", "op://Engineering/ProdDB/url"],
                Response::failure(
                    1,
                    "[ERROR] 2026/04/17 14:00:00 item \"ProdDB\" not found in vault \"Engineering\"\n",
                ),
            )
            .install(dir.path());
        let b = backend(&mock, None);
        let uri = BackendUri::parse("1password-personal://Engineering/ProdDB/url").unwrap();
        let err = b.get(&uri).await.unwrap_err();
        let msg = format!("{err:#}");
        assert!(msg.contains("not found"), "stderr propagates: {msg}");
        assert!(msg.contains("ProdDB"), "uri in context: {msg}");
        assert!(msg.contains("1password-personal"), "instance in context: {msg}");
    }

    #[tokio::test]
    async fn get_fails_fast_when_binary_missing() {
        let b = backend_with_nonexistent_op();
        let uri = BackendUri::parse("1password-personal://V/I/F").unwrap();
        let err = b.get(&uri).await.unwrap_err();
        let msg = format!("{err:#}");
        assert!(msg.contains("1password-personal"), "instance in context: {msg}");
    }

    // Non-UTF-8 response bodies: the strict harness's `Response.stdout:
    // String` field is UTF-8-only by construction, so this path stays on
    // the raw `install_mock` API. If a future backend routinely needs
    // non-UTF-8 stdout, extend the harness with `stdout_bytes: Vec<u8>`
    // — YAGNI until then. See v0.2.3 aws-ssm retrofit for precedent.
    #[tokio::test]
    async fn get_non_utf8_response_errors_with_context() {
        let dir = TempDir::new().unwrap();
        // Octal escapes (POSIX), not \xFF (bash-specific).
        let mock = secretenv_testing::install_mock(
            dir.path(),
            "op",
            r#"
if [ "$1" = "read" ]; then
  printf '\377\376\375\374'
  exit 0
fi
exit 1
"#,
        );
        let b = backend(&mock, None);
        let uri = BackendUri::parse("1password-personal://V/I/F").unwrap();
        let err = b.get(&uri).await.unwrap_err();
        let msg = format!("{err:#}");
        assert!(msg.contains("non-UTF-8"), "specific error: {msg}");
        assert!(msg.contains("1password-personal"), "instance in context: {msg}");
    }

    // ---- set, delete, list ----

    #[tokio::test]
    async fn set_succeeds_on_zero_exit() {
        let dir = TempDir::new().unwrap();
        let mock = StrictMock::new("op")
            .on(&["item", "edit", "I", "F=new-secret", "--vault", "V"], Response::success(""))
            .install(dir.path());
        let b = backend(&mock, None);
        let uri = BackendUri::parse("1password-personal://V/I/F").unwrap();
        b.set(&uri, "new-secret").await.unwrap();
    }

    #[tokio::test]
    async fn set_propagates_item_not_found_error() {
        let dir = TempDir::new().unwrap();
        let mock = StrictMock::new("op")
            .on(
                &["item", "edit", "I", "F=v", "--vault", "V"],
                Response::failure(1, "[ERROR] item \"I\" not found in vault \"V\"\n"),
            )
            .install(dir.path());
        let b = backend(&mock, None);
        let uri = BackendUri::parse("1password-personal://V/I/F").unwrap();
        let err = b.set(&uri, "v").await.unwrap_err();
        assert!(format!("{err:#}").contains("not found"));
    }

    #[tokio::test]
    async fn delete_runs_edit_with_empty_value() {
        // Under the strict harness, the declared argv `F=` (empty
        // assignment) is enforced by exact match — no side-channel log
        // needed. A regression that passed `F=foo` or omitted the
        // assignment would produce a no-match (exit 97) and fail the
        // test with a clear diagnostic.
        let dir = TempDir::new().unwrap();
        let mock = StrictMock::new("op")
            .on(&["item", "edit", "I", "F=", "--vault", "V"], Response::success(""))
            .install(dir.path());
        let b = backend(&mock, None);
        let uri = BackendUri::parse("1password-personal://V/I/F").unwrap();
        b.delete(&uri).await.unwrap();
    }

    #[tokio::test]
    async fn list_parses_toml_registry_document() {
        let dir = TempDir::new().unwrap();
        let body = "stripe-key = \"aws-ssm-prod:///prod/stripe\"\n\
                    db-url = \"1password-personal://Engineering/ProdDB/url\"\n";
        let mock = StrictMock::new("op")
            .on(&["read", "op://Shared/Registry/content"], Response::success(body))
            .install(dir.path());
        let b = backend(&mock, None);
        let uri = BackendUri::parse("1password-personal://Shared/Registry/content").unwrap();
        let mut entries = b.list(&uri).await.unwrap();
        entries.sort_by(|a, b| a.0.cmp(&b.0));
        assert_eq!(
            entries,
            vec![
                ("db-url".to_owned(), "1password-personal://Engineering/ProdDB/url".to_owned(),),
                ("stripe-key".to_owned(), "aws-ssm-prod:///prod/stripe".to_owned()),
            ]
        );
    }

    #[tokio::test]
    async fn list_errors_when_body_is_not_flat_toml() {
        let dir = TempDir::new().unwrap();
        let mock = StrictMock::new("op")
            .on(&["read", "op://V/I/F"], Response::success("[sub]\nkey = \"value\"\n"))
            .install(dir.path());
        let b = backend(&mock, None);
        let uri = BackendUri::parse("1password-personal://V/I/F").unwrap();
        let err = b.list(&uri).await.unwrap_err();
        let msg = format!("{err:#}");
        assert!(msg.contains("flat TOML") || msg.contains("string"), "specific error: {msg}");
    }

    // ---- check ----

    #[tokio::test]
    async fn check_returns_cli_missing_when_binary_not_found() {
        let b = backend_with_nonexistent_op();
        match b.check().await {
            BackendStatus::CliMissing { cli_name, install_hint } => {
                assert_eq!(cli_name, "op");
                assert!(
                    install_hint.contains("1password-cli") || install_hint.contains("developer")
                );
            }
            other => panic!("expected CliMissing, got {other:?}"),
        }
    }

    #[tokio::test]
    async fn check_returns_ok_when_version_and_whoami_succeed() {
        let dir = TempDir::new().unwrap();
        let mock = StrictMock::new("op")
            .on(&["--version"], Response::success("2.30.0\n"))
            .on(
                &["whoami", "--format=json", "--account", "my.1password.com"],
                Response::success("{\"url\":\"my.1password.com\",\"email\":\"me@example.com\"}\n"),
            )
            .install(dir.path());
        let b = backend(&mock, Some("my.1password.com"));
        match b.check().await {
            BackendStatus::Ok { cli_version, identity } => {
                assert!(cli_version.contains("2.30.0"));
                assert!(identity.contains("account=my.1password.com"));
                assert!(identity.contains("email=me@example.com"));
            }
            other => panic!("expected Ok, got {other:?}"),
        }
    }

    #[tokio::test]
    async fn check_returns_not_authenticated_on_sign_in_error() {
        let dir = TempDir::new().unwrap();
        let mock = StrictMock::new("op")
            .on(&["--version"], Response::success("2.30.0\n"))
            .on(
                &["whoami", "--format=json"],
                Response::failure(
                    1,
                    "[ERROR] You are not signed in. Run \"op signin\" to authenticate and try again.\n",
                ),
            )
            .install(dir.path());
        let b = backend(&mock, None);
        match b.check().await {
            BackendStatus::NotAuthenticated { hint } => {
                assert!(hint.contains("op signin"), "hint names signin: {hint}");
                assert!(hint.contains("not signed in"), "stderr in hint: {hint}");
            }
            other => panic!("expected NotAuthenticated, got {other:?}"),
        }
    }

    #[tokio::test]
    async fn check_signin_hint_includes_account_when_configured() {
        let dir = TempDir::new().unwrap();
        let mock = StrictMock::new("op")
            .on(&["--version"], Response::success("2.30.0\n"))
            .on(
                &["whoami", "--format=json", "--account", "myteam.1password.com"],
                Response::failure(1, "[ERROR] You are not signed in.\n"),
            )
            .install(dir.path());
        let b = backend(&mock, Some("myteam.1password.com"));
        match b.check().await {
            BackendStatus::NotAuthenticated { hint } => {
                assert!(
                    hint.contains("--account myteam.1password.com"),
                    "hint tailored to configured account: {hint}"
                );
            }
            other => panic!("expected NotAuthenticated, got {other:?}"),
        }
    }

    // ---- drift-catch regression locks ----

    // Guard against a regression that drops `--account <X>` from the argv
    // when the backend was configured with an account. The declared argv
    // intentionally omits the `--account` suffix; the real backend always
    // appends it when `op_account` is Some. A no-match (exit 97) with a
    // "strict-mock-no-match" stderr is the desired test outcome —
    // indicating account pass-through is still active.
    #[tokio::test]
    async fn get_drift_catch_rejects_missing_account_flag() {
        let dir = TempDir::new().unwrap();
        let mock = StrictMock::new("op")
            // Declared WITHOUT the `--account` tail. Real backend emits it.
            .on(&["read", "op://V/I/F"], Response::success("never-matches\n"))
            .install(dir.path());
        let b = backend(&mock, Some("myteam.1password.com"));
        let uri = BackendUri::parse("1password-personal://V/I/F").unwrap();
        let err = b.get(&uri).await.unwrap_err();
        let msg = format!("{err:#}");
        assert!(
            msg.contains("strict-mock-no-match") || msg.contains("not found"),
            "no-match diagnostic or stderr propagated: {msg}"
        );
    }

    // ---- v0.4 Phase 3: op_unsafe_set default-off lock ----

    #[tokio::test]
    async fn set_refuses_when_op_unsafe_set_is_false() {
        // The default-off behavior is the security improvement: rather
        // than silently passing the secret through argv (CV-1),
        // secretenv refuses unless the operator explicitly opts in.
        // The error message names the field and the exposure so the
        // operator can make an informed call.
        let dir = TempDir::new().unwrap();
        // Mock would succeed if reached, but the safe-default backend
        // shouldn't even invoke `op` — the bail! short-circuits.
        let mock = StrictMock::new("op")
            .on(&["item", "edit", "I", "F=v", "--vault", "V"], Response::success(""))
            .install(dir.path());
        let b = backend_safe_default(&mock);
        let uri = BackendUri::parse("1password-personal://V/I/F").unwrap();
        let err = b.set(&uri, "v").await.unwrap_err();
        let msg = format!("{err:#}");
        assert!(msg.contains("op_unsafe_set"), "names the opt-in field: {msg}");
        assert!(msg.contains("1password-personal"), "names instance: {msg}");
        assert!(msg.contains("argv"), "explains exposure: {msg}");
    }

    #[tokio::test]
    async fn set_proceeds_when_op_unsafe_set_is_true() {
        // The opt-in path uses the existing argv-based behavior,
        // logged with a warning. The strict mock asserts the same
        // argv as before — proves the opt-in restores prior shape.
        let dir = TempDir::new().unwrap();
        let mock = StrictMock::new("op")
            .on(&["item", "edit", "I", "F=new-secret", "--vault", "V"], Response::success(""))
            .install(dir.path());
        let mut b = backend_safe_default(&mock);
        b.op_unsafe_set = true;
        let uri = BackendUri::parse("1password-personal://V/I/F").unwrap();
        b.set(&uri, "new-secret").await.unwrap();
    }

    // Guard against a regression that drops the `--vault <V>` flag from
    // set()'s argv — without it, `op item edit` would target whatever
    // vault the user is defaulted to rather than the one the URI names.
    // Declared argv omits `--vault V`; real backend always appends it.
    #[tokio::test]
    async fn set_drift_catch_rejects_missing_vault_flag() {
        let dir = TempDir::new().unwrap();
        let mock = StrictMock::new("op")
            // Declared WITHOUT the `--vault V` tail. Real backend emits it.
            .on(&["item", "edit", "I", "F=new-secret"], Response::success(""))
            .install(dir.path());
        let b = backend(&mock, None);
        let uri = BackendUri::parse("1password-personal://V/I/F").unwrap();
        let err = b.set(&uri, "new-secret").await.unwrap_err();
        let msg = format!("{err:#}");
        assert!(msg.contains("strict-mock-no-match"), "no-match diagnostic: {msg}");
    }
}