diff --git a/CHANGELOG.md b/CHANGELOG.md index 342d2e3..ade9ab5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,30 @@ All notable changes to SecretEnv are documented here. The format follows [Keep a Alongside the standard sections (Added, Changed, Deprecated, Removed, Fixed, Security), releases from v0.14.0 onward use a `Known limitations` subsection for behavior that ships honestly but is incomplete by design. +## [0.20.0] - 2026-08-01 + +Value-integrity release. Five backends silently corrupted secrets on read; `set` was never affected, so no stored data is damaged and no migration is required. Live-backend smoke gains section 37, a byte-exact round-trip matrix covering 14 of 15 backends. + +### Fixed +- **macOS Keychain returned multi-line and non-ASCII values hex-encoded (data corruption).** `security find-*-password -w` silently hex-encodes its output, with no marker, whenever the value contains any byte outside printable ASCII — a newline, a tab, or any non-ASCII character. The backend returned that payload verbatim, so every PEM block, certificate, SSH key and accented passphrase came back corrupted on every macOS install. `get` now takes `security`'s own declaration of the encoding: shape decides only whether to *ask*, and `find-*-password -g` decides what the value *is*. Encoding is never inferred from shape, because `deadbeefcafe0123` is both an ordinary API key and valid hex. When `-g` cannot determine the encoding, `get` refuses rather than returning possibly-corrupt bytes; a non-UTF-8 item now errors cleanly instead of being lossily converted. Note that `-g` prints the password to stderr, so that stream is deliberately never routed into an error message, a log line, or a span field. +- **Vault, OpenBao, GCP and Azure truncated the final byte of any value ending in a newline (data corruption).** All four ended `get()` with `strip_suffix('\n')`, which is correct only when the CLI appends a newline of its own. `vault kv get -field=`, `bao kv get -field=` and `gcloud secrets versions access` all write the value's bytes and append nothing when stdout is a pipe; Azure parses the value out of `--output json`, where no CLI newline exists at all. The strip therefore ate the secret's own last byte. PEM blocks, certificates and SSH keys are conventionally stored with a trailing newline and are rejected by `openssl` / `ssh-keygen` without it, so this broke exactly the values most likely to be multi-line. All four now return the bytes verbatim. + +### Changed +- Values fetched from `keychain`, `vault`, `openbao`, `gcp` and `azure` may now end in a trailing newline where previous versions truncated it. This is the corrected behavior, but it is a change in returned bytes — consumers that compensated for the truncation should drop the workaround. + +### Added +- Live smoke section 37, `value encoding — multi-line / UTF-8 / trailing-newline / hex-shaped byte round-trip`: 104 assertions across 14 backends. Each value is seeded with the backend's native CLI, then read back twice — natively and through `secretenv` — so a backend that cannot hold the bytes records SKIP instead of being misreported as SecretEnv corruption. Includes a `hex-shaped` case (`deadbeefcafe0123`) that fails if anyone reintroduces shape-inferred decoding, and a cf-kv guard locking in that `wrangler kv key get --text` *does* append a newline and its strip is correct. +- New `assert_value_bytes` / `assert_native_holds` harness assertions comparing with `cmp` rather than `grep`. Every pre-existing assertion was substring-based and therefore blind to a lost trailing newline or an undecoded hex payload — which is why a fully green 810-assertion matrix coexisted with corruption on five backends. +- Per-backend regression tests (`get_preserves_legitimate_trailing_newline`) on vault, openbao, gcp and azure, plus a macOS-only `live` module in the keychain backend that exercises the real `security` binary via a self-cleaning per-PID item. + +### Known limitations +- **Doppler rejects multi-line values** at the API; the smoke records this as an explicit SKIP rather than a failure. Not a SecretEnv defect. +- **1Password truncates a trailing newline at write time** — `op item create` stores `abc` when given `abc\n`. SecretEnv reads back faithfully what 1Password holds; the loss happens in `op`, not in SecretEnv. +- **Keeper is not covered by section 37.** Seeding it requires `record-add` with a record type and folder, which would mutate the operator's real vault. Both read paths were verified instead: `--format=json` extracts from parsed JSON and applies no strip, and `keeper get --format=password` was measured live to append a newline, so its strip of exactly one is correct. Recorded as an explicit SKIP so the gap stays visible. + +### Internal +- Mock fixtures for vault, openbao, gcp and azure previously fed `"value\n"`, modelling those CLIs as appending a trailing newline they do not emit. The fixtures were corrected to match measured behavior rather than flipping the assertions, since the fixtures — not the assertions — encoded the wrong model. + ## [0.19.0] - 2026-06-14 Second non-backend hardening minor (hardening #2), consuming the v0.18 carry-forward queue and the deferred post-release documentation phase; no new backends (total 15), no new crates (workspace count 24); live-backend smoke 779 PASS / 0 FAIL / 2 expected SKIP across all 15 backends. diff --git a/Cargo.lock b/Cargo.lock index 3a7d65a..34ce13c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -72,9 +72,9 @@ dependencies = [ [[package]] name = "anyhow" -version = "1.0.102" +version = "1.0.104" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7f202df86484c868dbad7eaa557ef785d5c66295e41b460ef922eca0723b842c" +checksum = "330a5ed07fa54e4702c9d6c4174f74427fc0ef6e214bbd677ae50a5099946470" [[package]] name = "assert_cmd" @@ -1420,7 +1420,7 @@ dependencies = [ [[package]] name = "secretenv" -version = "0.19.0" +version = "0.20.0" dependencies = [ "anyhow", "assert_cmd", @@ -1463,7 +1463,7 @@ dependencies = [ [[package]] name = "secretenv-backend-1password" -version = "0.19.0" +version = "0.20.0" dependencies = [ "anyhow", "async-trait", @@ -1479,7 +1479,7 @@ dependencies = [ [[package]] name = "secretenv-backend-aws-secrets" -version = "0.19.0" +version = "0.20.0" dependencies = [ "anyhow", "async-trait", @@ -1494,7 +1494,7 @@ dependencies = [ [[package]] name = "secretenv-backend-aws-ssm" -version = "0.19.0" +version = "0.20.0" dependencies = [ "anyhow", "async-trait", @@ -1509,7 +1509,7 @@ dependencies = [ [[package]] name = "secretenv-backend-azure" -version = "0.19.0" +version = "0.20.0" dependencies = [ "anyhow", "async-trait", @@ -1525,7 +1525,7 @@ dependencies = [ [[package]] name = "secretenv-backend-bitwarden-sm" -version = "0.19.0" +version = "0.20.0" dependencies = [ "anyhow", "async-trait", @@ -1541,7 +1541,7 @@ dependencies = [ [[package]] name = "secretenv-backend-cf-kv" -version = "0.19.0" +version = "0.20.0" dependencies = [ "anyhow", "async-trait", @@ -1557,7 +1557,7 @@ dependencies = [ [[package]] name = "secretenv-backend-conjur" -version = "0.19.0" +version = "0.20.0" dependencies = [ "anyhow", "async-trait", @@ -1573,7 +1573,7 @@ dependencies = [ [[package]] name = "secretenv-backend-doppler" -version = "0.19.0" +version = "0.20.0" dependencies = [ "anyhow", "async-trait", @@ -1589,7 +1589,7 @@ dependencies = [ [[package]] name = "secretenv-backend-gcp" -version = "0.19.0" +version = "0.20.0" dependencies = [ "anyhow", "async-trait", @@ -1604,7 +1604,7 @@ dependencies = [ [[package]] name = "secretenv-backend-infisical" -version = "0.19.0" +version = "0.20.0" dependencies = [ "anyhow", "async-trait", @@ -1619,7 +1619,7 @@ dependencies = [ [[package]] name = "secretenv-backend-keeper" -version = "0.19.0" +version = "0.20.0" dependencies = [ "anyhow", "async-trait", @@ -1635,7 +1635,7 @@ dependencies = [ [[package]] name = "secretenv-backend-keychain" -version = "0.19.0" +version = "0.20.0" dependencies = [ "anyhow", "async-trait", @@ -1649,7 +1649,7 @@ dependencies = [ [[package]] name = "secretenv-backend-local" -version = "0.19.0" +version = "0.20.0" dependencies = [ "anyhow", "async-trait", @@ -1661,7 +1661,7 @@ dependencies = [ [[package]] name = "secretenv-backend-openbao" -version = "0.19.0" +version = "0.20.0" dependencies = [ "anyhow", "async-trait", @@ -1676,7 +1676,7 @@ dependencies = [ [[package]] name = "secretenv-backend-vault" -version = "0.19.0" +version = "0.20.0" dependencies = [ "anyhow", "async-trait", @@ -1691,7 +1691,7 @@ dependencies = [ [[package]] name = "secretenv-backends-init" -version = "0.19.0" +version = "0.20.0" dependencies = [ "anyhow", "secretenv-backend-1password", @@ -1714,7 +1714,7 @@ dependencies = [ [[package]] name = "secretenv-core" -version = "0.19.0" +version = "0.20.0" dependencies = [ "aho-corasick", "anyhow", @@ -1737,7 +1737,7 @@ dependencies = [ [[package]] name = "secretenv-mcp" -version = "0.19.0" +version = "0.20.0" dependencies = [ "anyhow", "directories", @@ -1764,7 +1764,7 @@ dependencies = [ [[package]] name = "secretenv-mcp-config" -version = "0.19.0" +version = "0.20.0" dependencies = [ "anyhow", "serde", @@ -1773,7 +1773,7 @@ dependencies = [ [[package]] name = "secretenv-migrate" -version = "0.19.0" +version = "0.20.0" dependencies = [ "anyhow", "secretenv-core", @@ -1785,7 +1785,7 @@ dependencies = [ [[package]] name = "secretenv-registry-mutate" -version = "0.19.0" +version = "0.20.0" dependencies = [ "anyhow", "secretenv-core", @@ -1794,7 +1794,7 @@ dependencies = [ [[package]] name = "secretenv-telemetry" -version = "0.19.0" +version = "0.20.0" dependencies = [ "getrandom 0.2.17", "hostname", @@ -1814,7 +1814,7 @@ dependencies = [ [[package]] name = "secretenv-testing" -version = "0.19.0" +version = "0.20.0" dependencies = [ "tempfile", ] diff --git a/Cargo.toml b/Cargo.toml index 6cfad11..f5aa14b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -28,7 +28,7 @@ members = [ ] [workspace.package] -version = "0.19.0" +version = "0.20.0" edition = "2021" license = "AGPL-3.0-only" repository = "https://github.com/TechAlchemistX/secretenv" @@ -46,28 +46,28 @@ readme = "README.md" # (`secretenv-core`, `secretenv-backend-*`). The CLI is the exception: it # lives at `crates/secretenv-cli/` but publishes as `secretenv` so # `cargo install secretenv` and the binary name line up. -secretenv-core = { path = "crates/secretenv-core", version = "0.19.0", features = ["value-access"] } -secretenv-telemetry = { path = "crates/secretenv-telemetry", version = "0.19.0" } -secretenv-migrate = { path = "crates/secretenv-migrate", version = "0.19.0" } -secretenv-registry-mutate = { path = "crates/secretenv-registry-mutate", version = "0.19.0" } -secretenv-mcp-config = { path = "crates/secretenv-mcp-config", version = "0.19.0" } -secretenv-backends-init = { path = "crates/secretenv-backends-init", version = "0.19.0" } -secretenv-mcp = { path = "crates/secretenv-mcp", version = "0.19.0" } -secretenv-backend-local = { path = "crates/backends/secretenv-backend-local", version = "0.19.0" } -secretenv-backend-aws-ssm = { path = "crates/backends/secretenv-backend-aws-ssm", version = "0.19.0" } -secretenv-backend-1password = { path = "crates/backends/secretenv-backend-1password", version = "0.19.0" } -secretenv-backend-vault = { path = "crates/backends/secretenv-backend-vault", version = "0.19.0" } -secretenv-backend-aws-secrets = { path = "crates/backends/secretenv-backend-aws-secrets", version = "0.19.0" } -secretenv-backend-gcp = { path = "crates/backends/secretenv-backend-gcp", version = "0.19.0" } -secretenv-backend-azure = { path = "crates/backends/secretenv-backend-azure", version = "0.19.0" } -secretenv-backend-keychain = { path = "crates/backends/secretenv-backend-keychain", version = "0.19.0" } -secretenv-backend-doppler = { path = "crates/backends/secretenv-backend-doppler", version = "0.19.0" } -secretenv-backend-infisical = { path = "crates/backends/secretenv-backend-infisical", version = "0.19.0" } -secretenv-backend-keeper = { path = "crates/backends/secretenv-backend-keeper", version = "0.19.0" } -secretenv-backend-cf-kv = { path = "crates/backends/secretenv-backend-cf-kv", version = "0.19.0" } -secretenv-backend-openbao = { path = "crates/backends/secretenv-backend-openbao", version = "0.19.0" } -secretenv-backend-conjur = { path = "crates/backends/secretenv-backend-conjur", version = "0.19.0" } -secretenv-backend-bitwarden-sm = { path = "crates/backends/secretenv-backend-bitwarden-sm", version = "0.19.0" } +secretenv-core = { path = "crates/secretenv-core", version = "0.20.0", features = ["value-access"] } +secretenv-telemetry = { path = "crates/secretenv-telemetry", version = "0.20.0" } +secretenv-migrate = { path = "crates/secretenv-migrate", version = "0.20.0" } +secretenv-registry-mutate = { path = "crates/secretenv-registry-mutate", version = "0.20.0" } +secretenv-mcp-config = { path = "crates/secretenv-mcp-config", version = "0.20.0" } +secretenv-backends-init = { path = "crates/secretenv-backends-init", version = "0.20.0" } +secretenv-mcp = { path = "crates/secretenv-mcp", version = "0.20.0" } +secretenv-backend-local = { path = "crates/backends/secretenv-backend-local", version = "0.20.0" } +secretenv-backend-aws-ssm = { path = "crates/backends/secretenv-backend-aws-ssm", version = "0.20.0" } +secretenv-backend-1password = { path = "crates/backends/secretenv-backend-1password", version = "0.20.0" } +secretenv-backend-vault = { path = "crates/backends/secretenv-backend-vault", version = "0.20.0" } +secretenv-backend-aws-secrets = { path = "crates/backends/secretenv-backend-aws-secrets", version = "0.20.0" } +secretenv-backend-gcp = { path = "crates/backends/secretenv-backend-gcp", version = "0.20.0" } +secretenv-backend-azure = { path = "crates/backends/secretenv-backend-azure", version = "0.20.0" } +secretenv-backend-keychain = { path = "crates/backends/secretenv-backend-keychain", version = "0.20.0" } +secretenv-backend-doppler = { path = "crates/backends/secretenv-backend-doppler", version = "0.20.0" } +secretenv-backend-infisical = { path = "crates/backends/secretenv-backend-infisical", version = "0.20.0" } +secretenv-backend-keeper = { path = "crates/backends/secretenv-backend-keeper", version = "0.20.0" } +secretenv-backend-cf-kv = { path = "crates/backends/secretenv-backend-cf-kv", version = "0.20.0" } +secretenv-backend-openbao = { path = "crates/backends/secretenv-backend-openbao", version = "0.20.0" } +secretenv-backend-conjur = { path = "crates/backends/secretenv-backend-conjur", version = "0.20.0" } +secretenv-backend-bitwarden-sm = { path = "crates/backends/secretenv-backend-bitwarden-sm", version = "0.20.0" } # Unpublished test harness — path-only, no version. Revisit publishing once # the API has proven itself across Vault + AWS Secrets backends (v0.3+). secretenv-testing = { path = "crates/secretenv-testing" } diff --git a/crates/backends/secretenv-backend-azure/src/lib.rs b/crates/backends/secretenv-backend-azure/src/lib.rs index 1dcf320..cad8bf6 100644 --- a/crates/backends/secretenv-backend-azure/src/lib.rs +++ b/crates/backends/secretenv-backend-azure/src/lib.rs @@ -300,7 +300,17 @@ impl AzureBackend { uri.raw ) })?; - Ok(value.strip_suffix('\n').unwrap_or(&value).to_owned()) + // Do NOT strip a trailing newline here. + // + // This value came out of `az ... --output json`, parsed above. + // A JSON string field carries the secret's exact bytes; there + // is no CLI-appended newline to remove, so the strip could only + // ever destroy data — it ate the final byte of any value ending + // in a newline (PEM, certs, SSH keys). This backend was the + // clearest case of the six: unlike the raw-stdout backends, + // no trailing newline is ever added on this path at all. + // Verified live (v0.19.1). + Ok(value) } } @@ -988,16 +998,41 @@ mod tests { } #[tokio::test] - async fn get_strips_single_trailing_newline() { + async fn get_returns_json_value_verbatim() { + // The value comes out of `az ... --output json`, so it carries + // the secret's exact bytes — there is no CLI-appended newline + // to remove. The old code stripped one anyway, which could only + // ever destroy data. let dir = TempDir::new().unwrap(); let mock = StrictMock::new("az") - .on(&show_argv("multi-line"), Response::success("{\"value\":\"line1\\nline2\\n\"}\n")) + .on(&show_argv("multi-line"), Response::success("{\"value\":\"line1\\nline2\"}\n")) .install(dir.path()); let b = backend(&mock, None, None); let uri = BackendUri::parse("azure-prod:///multi-line").unwrap(); assert_eq!(b.get(&uri).await.unwrap().expose_secret(), "line1\nline2"); } + /// Regression: a value whose final byte is a legitimate newline must + /// survive. The old code stripped one '\n' unconditionally, silently + /// truncating every PEM block, certificate and SSH key. Fails on the + /// pre-fix code. + #[tokio::test] + async fn get_preserves_legitimate_trailing_newline() { + let dir = TempDir::new().unwrap(); + let mock = StrictMock::new("az") + .on( + &show_argv("pem"), + Response::success("{\"value\":\"-----BEGIN X-----\\nabc\\n-----END X-----\\n\"}\n"), + ) + .install(dir.path()); + let b = backend(&mock, None, None); + let uri = BackendUri::parse("azure-prod:///pem").unwrap(); + assert_eq!( + b.get(&uri).await.unwrap().expose_secret(), + "-----BEGIN X-----\nabc\n-----END X-----\n" + ); + } + #[tokio::test] async fn get_empty_value() { let dir = TempDir::new().unwrap(); diff --git a/crates/backends/secretenv-backend-gcp/src/lib.rs b/crates/backends/secretenv-backend-gcp/src/lib.rs index 7a8084b..2d1d214 100644 --- a/crates/backends/secretenv-backend-gcp/src/lib.rs +++ b/crates/backends/secretenv-backend-gcp/src/lib.rs @@ -193,7 +193,15 @@ impl GcpBackend { self.instance_name, uri.raw ) })?; - Ok(stdout.strip_suffix('\n').unwrap_or(&stdout).to_owned()) + // Do NOT strip a trailing newline here. + // + // `gcloud secrets versions access` writes the secret payload + // byte-for-byte and appends nothing. Stripping one '\n' ate the + // final byte of every value that legitimately ends in a + // newline — PEM blocks, certificates and SSH keys, which are + // conventionally stored with one and are rejected by openssl / + // ssh-keygen without it. Verified live (v0.19.1). + Ok(stdout) } } @@ -780,7 +788,7 @@ mod tests { async fn get_returns_secret_latest() { let dir = TempDir::new().unwrap(); let mock = StrictMock::new("gcloud") - .on(&get_argv("stripe_key", "latest"), Response::success("sk_live_abc\n")) + .on(&get_argv("stripe_key", "latest"), Response::success("sk_live_abc")) .install(dir.path()); let b = backend(&mock, None); let uri = BackendUri::parse("gcp-prod:///stripe_key").unwrap(); @@ -791,7 +799,7 @@ mod tests { async fn get_returns_secret_at_version_5() { let dir = TempDir::new().unwrap(); let mock = StrictMock::new("gcloud") - .on(&get_argv("stripe_key", "5"), Response::success("older\n")) + .on(&get_argv("stripe_key", "5"), Response::success("older")) .install(dir.path()); let b = backend(&mock, None); let uri = BackendUri::parse("gcp-prod:///stripe_key#version=5").unwrap(); @@ -799,22 +807,41 @@ mod tests { } #[tokio::test] - async fn get_strips_single_trailing_newline() { + async fn get_returns_payload_verbatim() { + // `gcloud secrets versions access` writes the payload + // byte-for-byte and appends nothing. These fixtures previously + // carried a trailing "\n" that gcloud never emits, which is what + // made the old strip look correct. let dir = TempDir::new().unwrap(); - // Two trailing newlines in — only the LAST one is stripped. let mock = StrictMock::new("gcloud") - .on(&get_argv("multi_line", "latest"), Response::success("line1\nline2\n")) + .on(&get_argv("multi_line", "latest"), Response::success("line1\nline2")) .install(dir.path()); let b = backend(&mock, None); let uri = BackendUri::parse("gcp-prod:///multi_line").unwrap(); assert_eq!(b.get(&uri).await.unwrap().expose_secret(), "line1\nline2"); } + /// Regression: a payload whose final byte is a legitimate newline + /// must survive. The old code stripped one '\n' unconditionally, + /// silently truncating every PEM block, certificate and SSH key. + /// Fails on the pre-fix code. + #[tokio::test] + async fn get_preserves_legitimate_trailing_newline() { + let pem = "-----BEGIN X-----\nabc\n-----END X-----\n"; + let dir = TempDir::new().unwrap(); + let mock = StrictMock::new("gcloud") + .on(&get_argv("pem_secret", "latest"), Response::success(pem)) + .install(dir.path()); + let b = backend(&mock, None); + let uri = BackendUri::parse("gcp-prod:///pem_secret").unwrap(); + assert_eq!(b.get(&uri).await.unwrap().expose_secret(), pem); + } + #[tokio::test] async fn get_empty_value_returns_empty_string() { let dir = TempDir::new().unwrap(); let mock = StrictMock::new("gcloud") - .on(&get_argv("empty_secret", "latest"), Response::success("\n")) + .on(&get_argv("empty_secret", "latest"), Response::success("")) .install(dir.path()); let b = backend(&mock, None); let uri = BackendUri::parse("gcp-prod:///empty_secret").unwrap(); diff --git a/crates/backends/secretenv-backend-keychain/src/lib.rs b/crates/backends/secretenv-backend-keychain/src/lib.rs index 8e900fd..732f3ba 100644 --- a/crates/backends/secretenv-backend-keychain/src/lib.rs +++ b/crates/backends/secretenv-backend-keychain/src/lib.rs @@ -189,6 +189,89 @@ impl KeychainBackend { fn unlock_hint_target(&self) -> &str { self.keychain_path.as_deref().unwrap_or("~/Library/Keychains/login.keychain-db") } + + /// Second half of [`Backend::get`] for payloads that are shaped + /// like hex. Re-reads the item with `-g`, which declares the + /// encoding explicitly, and returns the true value. + /// + /// `raw` is `-w`'s stdout with its trailing newline already + /// stripped. If `-g` reports no hex, `raw` was the real value all + /// along (the legitimately-hex-valued-secret case) and is returned + /// untouched. + /// + /// SECURITY: `-g` prints the password to *stderr*. Nothing in this + /// function may route that stream into an error message, a log + /// line, or a span field — hence the deliberately generic failure + /// text below, and no use of `operation_failure_message` here. + async fn resolve_hex_shaped_value( + &self, + uri: &BackendUri, + raw: &str, + ) -> Result> { + let (service, account) = Self::parse_path(uri)?; + let mut cmd = self.security_command(self.kind.find_subcommand()); + cmd.args(["-s", &service, "-a", &account, "-g"]); + self.append_keychain(&mut cmd); + + let output = cmd.output().await.with_context(|| { + format!( + "keychain backend '{}': failed to invoke '{} {} -g' to determine the value \ + encoding for URI '{}'", + self.instance_name, + self.security_bin, + self.kind.find_subcommand(), + uri.raw + ) + })?; + + if !output.status.success() { + // `-w` succeeded but `-g` didn't, so we cannot tell whether + // the payload is a hex encoding or a genuinely hex-valued + // secret. Refuse rather than risk handing back corrupted + // bytes — silent corruption is the failure mode this whole + // path exists to remove. + bail!( + "keychain backend '{}': read the item at URI '{}' but could not determine its \ + encoding — `{} -g` failed (exit {}), and the value is ambiguous without it. \ + Check the item's access control in Keychain Access and retry.", + self.instance_name, + uri.raw, + self.kind.find_subcommand(), + output.status.code().unwrap_or(-1), + ); + } + + let stderr = String::from_utf8_lossy(&output.stderr); + let Some(hex) = parse_declared_hex(&stderr) else { + // `-g` quoted the value instead of hex-declaring it: `-w`'s + // payload was the literal secret, which merely happened to + // look like hex. + return Ok(Secret::new(raw.to_owned())); + }; + + let bytes = decode_hex(hex).with_context(|| { + format!( + "keychain backend '{}': `{} -g` declared a hex-encoded value for URI '{}' that \ + is not valid hex", + self.instance_name, + self.kind.find_subcommand(), + uri.raw + ) + })?; + + String::from_utf8(bytes).map(Secret::new).map_err(|_| { + // Keychain items hold arbitrary bytes; SecretEnv secrets are + // UTF-8 strings. Refuse cleanly rather than lossily + // substituting replacement characters. No bytes in the + // message. + anyhow::anyhow!( + "keychain backend '{}': the item at URI '{}' holds bytes that are not valid \ + UTF-8 — SecretEnv secrets must be UTF-8 text", + self.instance_name, + uri.raw + ) + }) + } } /// Decode `%2F` / `%2f` → `/`. Any other percent-escape passes @@ -198,6 +281,58 @@ fn decode_percent_slash(s: &str) -> String { s.replace("%2F", "/").replace("%2f", "/") } +/// Could this `security -w` payload plausibly be a hex encoding? +/// +/// This is ONLY a gate on whether to spend a second spawn asking `-g` +/// for the authoritative answer — it must never decide the value. +/// `deadbeefcafe0123` is a perfectly ordinary API key and is +/// indistinguishable here from a hex-encoded one; `-g` is what tells +/// the two apart. See [`parse_declared_hex`]. +/// +/// `-w`'s own hex output is always lowercase, but we accept either +/// case: a false positive costs one extra spawn and `-g` corrects it, +/// whereas case-sensitivity here would be a subtle trap for no gain. +fn looks_hex_encoded(s: &str) -> bool { + !s.is_empty() && s.len() % 2 == 0 && s.bytes().all(|b| b.is_ascii_hexdigit()) +} + +/// Extract the hex payload `security find-*-password -g` declares on +/// stderr, if any. +/// +/// `-g` writes the password in one of two forms: +/// +/// ```text +/// password: "plain-value" +/// password: 0x2D2D2D0A… "-----BEGIN X-----\012…" +/// ``` +/// +/// Returns `Some(hex)` for the second form and `None` for the first — +/// `None` means "`security` says this value is literally what `-w` +/// printed". The quoted tail is deliberately never parsed: it is +/// absent for a value that is just a newline, and its embedded quotes +/// are unescaped (`password: "a"b"`), so `-w`'s stdout is the better +/// source for the plain case. +fn parse_declared_hex(stderr: &str) -> Option<&str> { + stderr.lines().find_map(|line| { + let hex = line.strip_prefix("password: 0x")?; + Some(hex.split_whitespace().next().unwrap_or("")) + }) +} + +/// Decode an even-length ASCII hex string to bytes. `None` on any +/// malformed input. +fn decode_hex(hex: &str) -> Option> { + if hex.is_empty() || hex.len() % 2 != 0 || !hex.bytes().all(|b| b.is_ascii_hexdigit()) { + return None; + } + // `chunks_exact(2)` over verified ASCII hex: each pair is valid + // UTF-8 and parses as a byte, so no casts and no truncation. + hex.as_bytes() + .chunks_exact(2) + .map(|pair| u8::from_str_radix(std::str::from_utf8(pair).ok()?, 16).ok()) + .collect() +} + #[async_trait] impl Backend for KeychainBackend { fn backend_type(&self) -> &str { @@ -355,8 +490,26 @@ impl Backend for KeychainBackend { self.instance_name, uri.raw ) })?; - // `security -w` always appends a single '\n'. Strip it. - Ok(Secret::new(stdout.strip_suffix('\n').unwrap_or(&stdout).to_owned())) + // `security -w` always appends a single '\n'. Strip exactly + // one — a value that legitimately ends in a newline always + // takes the hex path below (a newline is one of the bytes that + // forces hex), so on the plain path the payload provably + // contains no newline at all and this can never over-strip. + let raw = stdout.strip_suffix('\n').unwrap_or(&stdout); + + // `security -w` silently hex-encodes, with no marker, whenever + // the value contains a byte outside printable ASCII — a + // newline (PEM, SSH key), a tab, or any non-ASCII UTF-8 (an + // accented character in a passphrase). Returning that payload + // verbatim hands back a corrupted secret. + // + // Shape alone cannot tell the two apart: `deadbeefcafe0123` is + // an ordinary API key that is also valid hex. So shape only + // decides whether to ASK; `-g` decides what the value IS. + if !looks_hex_encoded(raw) { + return Ok(Secret::new(raw.to_owned())); + } + self.resolve_hex_shaped_value(uri, raw).await } async fn set(&self, uri: &BackendUri, value: &str) -> Result<()> { @@ -864,6 +1017,271 @@ mod tests { assert!(msg.contains("keychain"), "names backend label: {msg}"); } + // ---- get: hex-encoded values (`security -w` has no marker) ---- + + #[test] + fn looks_hex_encoded_gates_only_on_shape() { + assert!(looks_hex_encoded("2d2d0a")); + assert!(looks_hex_encoded("DEADBEEF"), "case-insensitive gate"); + assert!(looks_hex_encoded("deadbeefcafe0123"), "real key that is also hex-shaped"); + assert!(!looks_hex_encoded(""), "empty value is not a hex payload"); + assert!(!looks_hex_encoded("abc"), "odd length cannot be hex"); + assert!(!looks_hex_encoded("sk_live_x"), "non-hex characters"); + assert!(!looks_hex_encoded("dead beef"), "whitespace is not hex"); + } + + #[test] + fn parse_declared_hex_reads_securitys_own_declaration() { + assert_eq!(parse_declared_hex("password: 0x616263 \"abc\"\n"), Some("616263")); + // A value that is just a newline: hex is declared with NO + // quoted tail. The parser must not require one. + assert_eq!(parse_declared_hex("password: 0x0A \n"), Some("0A")); + // Plain form: no hex declared. + assert_eq!(parse_declared_hex("password: \"deadbeefcafe0123\"\n"), None); + // Empty value: neither hex nor quotes. + assert_eq!(parse_declared_hex("password: \n"), None); + // A value that merely *contains* the marker text must not be + // mistaken for a declaration — only a line start counts. + assert_eq!(parse_declared_hex("keychain: \"x\"\npassword: \"a password: 0xff b\"\n"), None); + } + + #[test] + fn decode_hex_rejects_malformed_input() { + assert_eq!(decode_hex("616263"), Some(b"abc".to_vec())); + assert_eq!(decode_hex("0A"), Some(vec![b'\n'])); + assert_eq!(decode_hex("abc"), None, "odd length"); + assert_eq!(decode_hex("zz"), None, "non-hex digits"); + assert_eq!(decode_hex(""), None); + } + + /// The regression test for the corruption bug: a multi-line PEM + /// comes back from `-w` hex-encoded with no marker, and `-g` is + /// what declares it. + #[tokio::test] + async fn get_decodes_hex_when_security_declares_it() { + let dir = TempDir::new().unwrap(); + let pem = "-----BEGIN X-----\nabc\n-----END X-----\n"; + let mock = StrictMock::new("security") + .on( + &["find-generic-password", "-s", "stripe", "-a", "prod", "-w"], + Response::success( + "2d2d2d2d2d424547494e20582d2d2d2d2d0a6162630a2d2d2d2d2d454e4420582d2d2d2d2d0a\n", + ), + ) + .on( + &["find-generic-password", "-s", "stripe", "-a", "prod", "-g"], + Response::success("").with_stderr( + "password: 0x2D2D2D2D2D424547494E20582D2D2D2D2D0A6162630A2D2D2D2D2D454E4420\ + 582D2D2D2D2D0A \"-----BEGIN X-----\\012abc\\012-----END X-----\\012\"\n", + ), + ) + .install(dir.path()); + let b = backend(&mock, None, Kind::GenericPassword); + let uri = BackendUri::parse("keychain-default://stripe/prod").unwrap(); + assert_eq!(b.get(&uri).await.unwrap().expose_secret(), pem); + } + + /// The more important of the two: a secret whose real value is + /// hex-shaped must survive untouched. This is what fails if anyone + /// later "fixes" the encoding by inferring it from shape. + #[tokio::test] + async fn get_leaves_legitimately_hex_valued_secret_alone() { + let dir = TempDir::new().unwrap(); + let mock = StrictMock::new("security") + .on( + &["find-generic-password", "-s", "stripe", "-a", "prod", "-w"], + Response::success("deadbeefcafe0123\n"), + ) + .on( + &["find-generic-password", "-s", "stripe", "-a", "prod", "-g"], + Response::success("").with_stderr("password: \"deadbeefcafe0123\"\n"), + ) + .install(dir.path()); + let b = backend(&mock, None, Kind::GenericPassword); + let uri = BackendUri::parse("keychain-default://stripe/prod").unwrap(); + assert_eq!(b.get(&uri).await.unwrap().expose_secret(), "deadbeefcafe0123"); + } + + /// A plain value must cost exactly one spawn. `StrictMock` fails + /// the test on any unregistered argv, so the absence of a `-g` rule + /// here is the assertion that `-g` was never invoked. + #[tokio::test] + async fn get_plain_value_never_invokes_dash_g() { + let dir = TempDir::new().unwrap(); + let mock = StrictMock::new("security") + .on( + &["find-generic-password", "-s", "stripe", "-a", "prod", "-w"], + Response::success("sk_live_not_hex_shaped\n"), + ) + .install(dir.path()); + let b = backend(&mock, None, Kind::GenericPassword); + let uri = BackendUri::parse("keychain-default://stripe/prod").unwrap(); + assert_eq!(b.get(&uri).await.unwrap().expose_secret(), "sk_live_not_hex_shaped"); + } + + #[tokio::test] + async fn get_appends_keychain_path_to_the_dash_g_probe_too() { + let dir = TempDir::new().unwrap(); + let path = "/tmp/custom.keychain-db"; + let mock = StrictMock::new("security") + .on( + &["find-generic-password", "-s", "stripe", "-a", "prod", "-w", path], + Response::success("0a0a\n"), + ) + .on( + &["find-generic-password", "-s", "stripe", "-a", "prod", "-g", path], + Response::success("").with_stderr("password: 0x0A0A \n"), + ) + .install(dir.path()); + let b = backend(&mock, Some(path), Kind::GenericPassword); + let uri = BackendUri::parse("keychain-default://stripe/prod").unwrap(); + assert_eq!(b.get(&uri).await.unwrap().expose_secret(), "\n\n"); + } + + #[tokio::test] + async fn get_refuses_when_encoding_cannot_be_determined() { + let dir = TempDir::new().unwrap(); + let mock = StrictMock::new("security") + .on( + &["find-generic-password", "-s", "stripe", "-a", "prod", "-w"], + Response::success("deadbeef\n"), + ) + .on( + &["find-generic-password", "-s", "stripe", "-a", "prod", "-g"], + Response::failure(25, "security: User interaction is not allowed.\n"), + ) + .install(dir.path()); + let b = backend(&mock, None, Kind::GenericPassword); + let uri = BackendUri::parse("keychain-default://stripe/prod").unwrap(); + let err = b.get(&uri).await.unwrap_err(); + let msg = format!("{err:#}"); + assert!(msg.contains("could not determine its encoding"), "names the cause: {msg}"); + assert!(msg.contains("keychain-default://stripe/prod"), "names URI: {msg}"); + } + + #[tokio::test] + async fn get_refuses_non_utf8_item_without_leaking_bytes() { + let dir = TempDir::new().unwrap(); + let mock = StrictMock::new("security") + .on( + &["find-generic-password", "-s", "stripe", "-a", "prod", "-w"], + Response::success("fffe\n"), + ) + .on( + &["find-generic-password", "-s", "stripe", "-a", "prod", "-g"], + Response::success("").with_stderr("password: 0xFFFE \n"), + ) + .install(dir.path()); + let b = backend(&mock, None, Kind::GenericPassword); + let uri = BackendUri::parse("keychain-default://stripe/prod").unwrap(); + let err = b.get(&uri).await.unwrap_err(); + let msg = format!("{err:#}"); + assert!(msg.contains("not valid UTF-8"), "names the cause: {msg}"); + } + + // ---- get: live `security` round-trips (macOS only) ---- + // + // Every test above drives `StrictMock`, which returns whatever the + // test declares — so none of them can observe the REAL encoding + // behaviour of `security`. That blind spot is exactly why the hex + // corruption survived a green suite. These four talk to the real + // binary. + // + // Each uses a service name unique to the process and deletes its + // items on the way out, so they never touch the user's own + // keychain entries. + + #[cfg(target_os = "macos")] + mod live { + use super::*; + + /// RAII guard: deletes the item even if the test panics. + struct LiveItem { + service: String, + } + + impl LiveItem { + /// Store `value` under a service name unique to this + /// process + `tag`, and return a guard that removes it. + fn store(tag: &str, value: &str) -> Self { + let service = format!("se-live-test-{}-{tag}", std::process::id()); + let status = std::process::Command::new("security") + .args(["add-generic-password", "-s", &service, "-a", "acct", "-w", value, "-U"]) + .status() + .expect("spawn security add-generic-password"); + assert!(status.success(), "failed to seed live keychain item for '{tag}'"); + Self { service } + } + + fn uri(&self) -> BackendUri { + BackendUri::parse(&format!("keychain-default://{}/acct", self.service)).unwrap() + } + } + + impl Drop for LiveItem { + fn drop(&mut self) { + let _ = std::process::Command::new("security") + .args(["delete-generic-password", "-s", &self.service, "-a", "acct"]) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .status(); + } + } + + fn live_backend() -> KeychainBackend { + KeychainBackend { + backend_type: "keychain", + instance_name: "keychain-default".to_owned(), + keychain_path: None, + kind: Kind::GenericPassword, + security_bin: "security".to_owned(), + timeout: DEFAULT_GET_TIMEOUT, + } + } + + async fn round_trip(tag: &str, value: &str) { + let item = LiveItem::store(tag, value); + let got = live_backend().get(&item.uri()).await.unwrap(); + assert_eq!(got.expose_secret(), value, "'{tag}' did not round-trip byte-identically"); + } + + /// The regression test. Fails on the pre-fix code, which + /// returns the undecoded hex. + #[tokio::test] + async fn multiline_pem_round_trips_through_real_security() { + round_trip("pem", "-----BEGIN X-----\nabc\n-----END X-----\n").await; + } + + /// The guard against a future shape-inference "fix". + #[tokio::test] + async fn hex_valued_secret_round_trips_through_real_security() { + round_trip("hexval", "deadbeefcafe0123").await; + } + + /// `security` also hex-encodes any non-ASCII byte, so ordinary + /// UTF-8 passphrases were corrupted by the same defect. + #[tokio::test] + async fn non_ascii_utf8_round_trips_through_real_security() { + round_trip("utf8", "café-ünïcode-π").await; + } + + /// A trailing newline must be preserved, and a value without + /// one must not gain one. + #[tokio::test] + async fn trailing_newline_is_preserved_exactly() { + round_trip("trailnl", "abc\n").await; + round_trip("notrailnl", "abc").await; + } + + /// `-g` renders the plain form as `password: "a"b"` — quotes + /// inside the value are NOT escaped. The parser must not be + /// confused by them. + #[tokio::test] + async fn quotes_and_backslashes_round_trip() { + round_trip("quotes", r#"he said "hi" \back\ 'end'"#).await; + } + } + // ---- set ---- #[tokio::test] diff --git a/crates/backends/secretenv-backend-openbao/src/lib.rs b/crates/backends/secretenv-backend-openbao/src/lib.rs index 0ee5151..f2b497e 100644 --- a/crates/backends/secretenv-backend-openbao/src/lib.rs +++ b/crates/backends/secretenv-backend-openbao/src/lib.rs @@ -241,9 +241,13 @@ impl OpenBaoBackend { self.instance_name, uri.raw ) })?; - // `-field=value` output ends with exactly one '\n'; strip it - // but keep any other trailing whitespace. - Ok(stdout.strip_suffix('\n').unwrap_or(&stdout).to_owned()) + // Do NOT strip a trailing newline here — see the equivalent + // note in the vault backend. `bao kv get -field=` writes the + // field's bytes verbatim and appends nothing when stdout is a + // pipe, so stripping one '\n' ate the final byte of every + // value that legitimately ends in a newline (PEM, certs, SSH + // keys). Verified live (v0.19.1). + Ok(stdout) } } @@ -889,10 +893,13 @@ mod tests { // ---- get ---- #[tokio::test] - async fn get_returns_trimmed_value() { + async fn get_returns_value_verbatim() { + // `bao kv get -field=` appends NOTHING when stdout is a pipe. + // These fixtures previously carried a trailing "\n" that real + // OpenBao never emits, which is what made the strip look right. let dir = TempDir::new().unwrap(); let mock = StrictMock::new("bao") - .on(&["kv", "get", "-field=value", "secret/myapp/db"], ok_no_ns("supersekrit\n")) + .on(&["kv", "get", "-field=value", "secret/myapp/db"], ok_no_ns("supersekrit")) .install(dir.path()); let b = backend(&mock, None); let uri = BackendUri::parse("openbao-dev://secret/myapp/db").unwrap(); @@ -903,7 +910,7 @@ mod tests { async fn get_empty_value_returns_empty_string() { let dir = TempDir::new().unwrap(); let mock = StrictMock::new("bao") - .on(&["kv", "get", "-field=value", "secret/myapp/empty"], ok_no_ns("\n")) + .on(&["kv", "get", "-field=value", "secret/myapp/empty"], ok_no_ns("")) .install(dir.path()); let b = backend(&mock, None); let uri = BackendUri::parse("openbao-dev://secret/myapp/empty").unwrap(); @@ -914,13 +921,29 @@ mod tests { async fn get_preserves_internal_newlines() { let dir = TempDir::new().unwrap(); let mock = StrictMock::new("bao") - .on(&["kv", "get", "-field=value", "secret/myapp/ws"], ok_no_ns("line1\nline2\n")) + .on(&["kv", "get", "-field=value", "secret/myapp/ws"], ok_no_ns("line1\nline2")) .install(dir.path()); let b = backend(&mock, None); let uri = BackendUri::parse("openbao-dev://secret/myapp/ws").unwrap(); assert_eq!(b.get(&uri).await.unwrap().expose_secret(), "line1\nline2"); } + /// Regression: a value whose final byte is a legitimate newline must + /// survive. The old code stripped one '\n' unconditionally, silently + /// truncating every PEM block, certificate and SSH key. Fails on the + /// pre-fix code. + #[tokio::test] + async fn get_preserves_legitimate_trailing_newline() { + let pem = "-----BEGIN X-----\nabc\n-----END X-----\n"; + let dir = TempDir::new().unwrap(); + let mock = StrictMock::new("bao") + .on(&["kv", "get", "-field=value", "secret/myapp/pem"], ok_no_ns(pem)) + .install(dir.path()); + let b = backend(&mock, None); + let uri = BackendUri::parse("openbao-dev://secret/myapp/pem").unwrap(); + assert_eq!(b.get(&uri).await.unwrap().expose_secret(), pem); + } + #[tokio::test] async fn get_not_found_wraps_bao_stderr() { let dir = TempDir::new().unwrap(); diff --git a/crates/backends/secretenv-backend-vault/src/lib.rs b/crates/backends/secretenv-backend-vault/src/lib.rs index b85c807..5a029f5 100644 --- a/crates/backends/secretenv-backend-vault/src/lib.rs +++ b/crates/backends/secretenv-backend-vault/src/lib.rs @@ -29,8 +29,11 @@ //! //! # Semantics //! -//! - [`get`](VaultBackend) runs `vault kv get -field=value ` -//! and trims exactly one trailing newline. +//! - [`get`](VaultBackend) runs `vault kv get -field=value ` and +//! returns the bytes verbatim. The CLI appends no trailing newline +//! when stdout is a pipe, so nothing is trimmed — trimming one here +//! truncated every value that legitimately ended in a newline (PEM +//! blocks, certificates, SSH keys). //! - [`set`](VaultBackend) runs `vault kv put value=-` and //! pipes the secret through child stdin. The value NEVER appears //! on argv. @@ -327,10 +330,23 @@ impl Backend for VaultBackend { self.instance_name, uri.raw ) })?; - // `-field=value` output ends with exactly one '\n'. Strip - // it but keep any other trailing whitespace (operators who - // deliberately stored trailing spaces deserve them back). - Ok(Secret::new(stdout.strip_suffix('\n').unwrap_or(&stdout).to_owned())) + // Do NOT strip a trailing newline here. + // + // `vault kv get -field=` writes the field's bytes verbatim + // and appends NOTHING when stdout is not a TTY — which it never + // is for us, since we always capture it via a pipe. The old + // code stripped one '\n' on the assumption that vault added + // one, which silently ate the final byte of every value that + // legitimately ends in a newline: PEM blocks, certificates and + // SSH keys, all of which are conventionally stored with a + // trailing newline and are rejected by openssl / ssh-keygen + // without it. + // + // Verified live (v0.19.1): `vault kv get -field=value` on a + // value with no trailing newline returns stdout with no + // trailing newline. Contrast aws-ssm's `--output text`, which + // DOES append one and therefore still strips. + Ok(Secret::new(stdout)) } async fn set(&self, uri: &BackendUri, value: &str) -> Result<()> { @@ -898,10 +914,14 @@ mod tests { // ---- get ---- #[tokio::test] - async fn get_returns_trimmed_value() { + async fn get_returns_value_verbatim() { + // `vault kv get -field=` writes the field's bytes and appends + // NOTHING when stdout is a pipe (which it always is here). These + // fixtures previously carried a trailing "\n" that real vault + // never emits, which is what made the strip look correct. let dir = TempDir::new().unwrap(); let mock = StrictMock::new("vault") - .on(&["kv", "get", "-field=value", "secret/myapp/db"], ok_no_ns("supersekrit\n")) + .on(&["kv", "get", "-field=value", "secret/myapp/db"], ok_no_ns("supersekrit")) .install(dir.path()); let b = backend(&mock, None); let uri = BackendUri::parse("vault-eng://secret/myapp/db").unwrap(); @@ -910,11 +930,9 @@ mod tests { #[tokio::test] async fn get_empty_value_returns_empty_string() { - // Vault prints "\n" when the value is empty; we strip the one - // trailing '\n' → "". let dir = TempDir::new().unwrap(); let mock = StrictMock::new("vault") - .on(&["kv", "get", "-field=value", "secret/myapp/empty"], ok_no_ns("\n")) + .on(&["kv", "get", "-field=value", "secret/myapp/empty"], ok_no_ns("")) .install(dir.path()); let b = backend(&mock, None); let uri = BackendUri::parse("vault-eng://secret/myapp/empty").unwrap(); @@ -923,16 +941,33 @@ mod tests { #[tokio::test] async fn get_preserves_internal_newlines() { - // Multi-line secret body: we only strip a single trailing '\n'. let dir = TempDir::new().unwrap(); let mock = StrictMock::new("vault") - .on(&["kv", "get", "-field=value", "secret/myapp/ws"], ok_no_ns("line1\nline2\n")) + .on(&["kv", "get", "-field=value", "secret/myapp/ws"], ok_no_ns("line1\nline2")) .install(dir.path()); let b = backend(&mock, None); let uri = BackendUri::parse("vault-eng://secret/myapp/ws").unwrap(); assert_eq!(b.get(&uri).await.unwrap().expose_secret(), "line1\nline2"); } + /// Regression: a value whose final byte is a legitimate newline + /// must come back with it. The old code stripped one '\n' + /// unconditionally, silently truncating every PEM block, + /// certificate and SSH key — all of which are conventionally stored + /// with a trailing newline and are rejected by openssl / ssh-keygen + /// without it. Fails on the pre-fix code. + #[tokio::test] + async fn get_preserves_legitimate_trailing_newline() { + let pem = "-----BEGIN X-----\nabc\n-----END X-----\n"; + let dir = TempDir::new().unwrap(); + let mock = StrictMock::new("vault") + .on(&["kv", "get", "-field=value", "secret/myapp/pem"], ok_no_ns(pem)) + .install(dir.path()); + let b = backend(&mock, None); + let uri = BackendUri::parse("vault-eng://secret/myapp/pem").unwrap(); + assert_eq!(b.get(&uri).await.unwrap().expose_secret(), pem); + } + #[tokio::test] async fn get_not_found_wraps_vault_stderr() { let dir = TempDir::new().unwrap(); diff --git a/crates/secretenv-mcp/Cargo.toml b/crates/secretenv-mcp/Cargo.toml index 356a2d9..f4ef6dd 100644 --- a/crates/secretenv-mcp/Cargo.toml +++ b/crates/secretenv-mcp/Cargo.toml @@ -22,7 +22,7 @@ readme.workspace = true # (`disallowed-types`) + `tests/boundary_test.rs`. See SEC-INV-02 + # build-plan §5. [dependencies] -secretenv-core = { path = "../secretenv-core", version = "0.19.0", default-features = false } +secretenv-core = { path = "../secretenv-core", version = "0.20.0", default-features = false } # `secretenv-backends-init` is the SHARED factory wiring used by both # the CLI and the MCP server. Pulled in for `doctor` + `resolve_status` # which need a live `BackendRegistry` at call time. Naming diff --git a/docs/backends/keychain.md b/docs/backends/keychain.md index e64a20a..fd07551 100644 --- a/docs/backends/keychain.md +++ b/docs/backends/keychain.md @@ -179,6 +179,34 @@ No fragment directives. Any `#...` fragment is rejected at URI-parse time. Not implemented. The Keychain has no version-history API. Items are overwritten in place on `set` with no audit trail. +## Value encoding + +Values round-trip byte-for-byte, including multi-line PEM blocks, SSH keys, tabs, and non-ASCII UTF-8. This needs explaining because the underlying CLI makes it non-obvious. + +`security find-*-password -w` **silently hex-encodes its output, with no marker**, whenever the value contains any byte outside printable ASCII — a newline, a tab, or any non-ASCII character: + +```console +$ security find-generic-password -s se-demo -a pem -w +2d2d2d2d2d424547494e20582d2d2d2d2d0a6162630a2d2d2d2d2d454e4420582d2d2d2d2d0a +``` + +SecretEnv cannot infer the encoding from that payload, because `deadbeefcafe0123` is both a perfectly ordinary API key and valid hex. Instead it re-reads the item with `-g`, which declares the encoding explicitly, and takes `security`'s own word for it: + +```text +password: "deadbeefcafe0123" → the literal value +password: 0x2D2D2D… "-----BEGIN X-----\012…" → hex-encoded, decode it +``` + +Practical consequences: + +- **A hex-shaped value costs one extra `security` spawn.** Plain values cost one spawn, as before. +- **Binary secrets are refused, not mangled.** Keychain items hold arbitrary bytes; SecretEnv secrets are UTF-8 text. A non-UTF-8 item fails with a clear error rather than being lossily converted. +- **If `-g` fails, `get` refuses.** When the encoding can't be determined, SecretEnv errors instead of returning bytes that may be corrupt. This usually means the item's ACL needs attention in Keychain Access. + +`set` is unaffected — it has always stored these values correctly. + +> Fixed in v0.20.0. Earlier versions returned the undecoded hex string for any such value. + ## Limitations - **Stdin discipline.** Every `security` invocation sets `stdin: Stdio::null()` to prevent hangs on locked keychains. diff --git a/scripts/smoke-test/run-tests.sh b/scripts/smoke-test/run-tests.sh index 75060c2..2d28818 100755 --- a/scripts/smoke-test/run-tests.sh +++ b/scripts/smoke-test/run-tests.sh @@ -132,6 +132,7 @@ SECTIONS=( # MCP boundary; v0.16 ships with local + SEC-INV-20 + value-grep # as the must-have surface. "35|v0.16 secretenv mcp — protocol smoke + SEC-INV-20 + value-grep|no" + "37|value encoding — multi-line / UTF-8 / trailing-newline / hex-shaped byte round-trip|yes" ) print_section_inventory() { @@ -342,6 +343,95 @@ assert_not_contains() { fi } +# ---- byte-exact value assertions (section 37) ----------------------------- +# +# Every assertion above is grep-based, which cannot see trailing +# whitespace, a lost final newline, or an undecoded hex payload. That +# blind spot is exactly how the v0.19 keychain hex-corruption bug +# survived a fully green 810-assertion matrix: `security -w` returns +# multi-line and non-ASCII values hex-encoded with no marker, and +# `assert_contains` on a substring still matched. +# +# `enc_seed` writes a value's EXACT bytes through a native CLI. +# Shell `$(cat f)` strips trailing newlines, so the value is injected +# via a placeholder that perl swaps for the file's literal contents. +# The placeholder deliberately avoids '@' — perl interpolates arrays +# inside double-quoted strings, so an '@@VAL@@' style marker silently +# stores itself instead of the value. +enc_seed() { + perl -e ' + my $f = shift @ARGV; + local $/; open(my $fh, "<", $f) or die "enc_seed open: $!"; my $v = <$fh>; + $v = "" unless defined $v; + my @cmd = map { my $a = $_; $a =~ s/__VAL__/$v/g; $a } @ARGV; + exec(@cmd) or exit 127; + ' "$@" +} + +# Read one alias back through `secretenv get` and compare byte-for-byte +# against the expected fixture. `get` appends exactly one newline to +# whatever it prints, so strip exactly one before comparing — stripping +# more would mask the very defect this section exists to catch. +assert_value_bytes() { + if [ "$SECTION_ACTIVE" = "0" ]; then return 0; fi + local name="$1" expected="$2" cfg="$3" alias="$4" log="$5" native="${6:-}" + local raw="${log%.log}.raw" + # If the native seed never landed, the backend rejected the value + # outright. Reading it back through SecretEnv then fails for a + # reason that has nothing to do with SecretEnv, so record SKIP + # rather than a FAIL that would misattribute a backend limitation + # to a product defect. + if [ -n "$native" ] && [ ! -f "$native.nativeok" ]; then + record "$name" "SKIP" \ + "backend did not hold these bytes at seed time — SecretEnv not exercised" + return + fi + if ! "$BIN" --config "$cfg" get "$alias" --yes > "$raw" 2>"$log"; then + record "$name" "FAIL" "get failed: $(head -1 "$log" | cut -c1-60)" + return + fi + perl -0777 -pe 's/\n\z//' "$raw" > "$raw.trim" + if cmp -s "$expected" "$raw.trim"; then + record "$name" "PASS" "byte-identical round-trip" + else + record "$name" "FAIL" \ + "expected=$(xxd -p "$expected" | tr -d '\n' | cut -c1-40) got=$(xxd -p "$raw.trim" | tr -d '\n' | cut -c1-40)" + fi +} + +# Same comparison, but against a NATIVE CLI readback rather than +# SecretEnv. Distinguishes "this backend cannot hold these bytes at +# all" (a documented limitation) from "SecretEnv corrupts them on read" +# (a defect). Without this split every mismatch reads as a SecretEnv +# bug, which would misattribute the failure to the wrong layer. +assert_native_holds() { + if [ "$SECTION_ACTIVE" = "0" ]; then return 0; fi + local name="$1" expected="$2" raw="$3" appends="$4" + if [ ! -s "$raw" ] && [ -s "$expected" ]; then + record "$name" "SKIP" "native seed/readback produced nothing — backend rejected the value" + return + fi + # `appends` says whether THIS CLI adds a newline of its own to the + # value it prints. It must be stated per backend, not guessed: + # accepting "raw OR raw-minus-one-newline" cannot distinguish a + # stored "abc\n" from a stored "abc" printed by a CLI that appends, + # and that ambiguity silently turns a seed-side truncation into a + # false "SecretEnv corrupts it" verdict. + local cmp_file="$raw" + if [ "$appends" = "yes" ]; then + perl -0777 -pe 's/\n\z//' "$raw" > "$raw.trim1" + cmp_file="$raw.trim1" + fi + if cmp -s "$expected" "$cmp_file"; then + : > "$raw.nativeok" + record "$name" "PASS" "native CLI preserves the bytes" + else + rm -f "$raw.nativeok" + record "$name" "SKIP" \ + "backend/CLI does not preserve these bytes — SecretEnv not implicated" + fi +} + echo "=== SecretEnv validation — $(date -u +%FT%TZ) ===" | tee -a "$SUMMARY" echo "binary: $("$BIN" --version)" | tee -a "$SUMMARY" echo "config: $CFG" | tee -a "$SUMMARY" @@ -4790,6 +4880,404 @@ SECF5_MANIFEST fi +# --------------------------------------------------------------- +# 37 — value encoding: byte-exact round-trip of awkward values +# --------------------------------------------------------------- +# Motivation: the v0.19 keychain hex-corruption defect. `security -w` +# hex-encodes its output, with no marker, for any value containing a +# byte outside printable ASCII. The backend returned that payload +# verbatim, so every multi-line secret (PEM, SSH key) and every +# non-ASCII value came back corrupted — silently, on every macOS +# install — while the full matrix stayed green, because no assertion +# anywhere compared bytes. +# +# Four values, chosen to catch distinct failure modes: +# pem multi-line, and ends in a newline +# utf8 non-ASCII (same root cause, wider blast radius) +# trailnl a value whose final byte is a legitimate newline +# hexval a real secret that happens to be valid hex — fails if +# anyone ever "fixes" an encoding by inferring it from shape +# +# Each value is seeded with the backend's NATIVE CLI, then read back +# twice: natively (does the backend even hold these bytes?) and through +# SecretEnv (does SecretEnv return them intact?). A native readback that +# already lost the bytes records SKIP, not FAIL — that is a backend +# limitation and SecretEnv is not implicated. +section_begin 37 "value encoding — byte-exact round-trip" +if [ "$SECTION_ACTIVE" = "1" ]; then + ENC="$RUNS/370-encoding" + rm -rf "$ENC"; mkdir -p "$ENC" + + printf -- '-----BEGIN X-----\nabc\n-----END X-----\n' > "$ENC/pem.expected" + printf -- 'caf\xc3\xa9-\xc3\xbcn\xc3\xafcode-\xcf\x80' > "$ENC/utf8.expected" + printf -- 'abc\n' > "$ENC/trailnl.expected" + printf -- 'deadbeefcafe0123' > "$ENC/hexval.expected" + ENC_VALUES="pem utf8 trailnl hexval" + + # Build a per-backend registry (one alias per value) + config. + # $1=name $2=backend TOML block (empty for local) $3=URI template with %V% + enc_mkenv() { + local name="$1" block="$2" tpl="$3" + local reg="$ENC/$name-registry.toml" v u + : > "$reg" + local uv + for v in $ENC_VALUES; do + uv=$(printf '%s' "$v" | tr 'a-z' 'A-Z') + u="${tpl//%V%/$v}" + u="${u//%U%/$uv}" + printf '%s = "%s"\n' "$v" "$u" >> "$reg" + done + { + printf '[registries.default]\nsources = ["local-main://%s"]\n\n' "$reg" + printf '[backends.local-main]\ntype = "local"\n\n' + printf '%s\n' "$block" + } > "$ENC/$name-config.toml" + echo "$ENC/$name-config.toml" + } + + # --- local ------------------------------------------------------ + for v in $ENC_VALUES; do + cp "$ENC/$v.expected" "$ENC/local-src-$v" + assert_native_holds "3701-$v local native holds bytes" \ + "$ENC/$v.expected" "$ENC/local-src-$v" "no" + done + ENC_CFG=$(enc_mkenv local "" "local-main://$ENC/local-src-%V%") + for v in $ENC_VALUES; do + assert_value_bytes "3702-$v local round-trip" \ + "$ENC/$v.expected" "$ENC_CFG" "$v" "$ENC/local-$v.log" "$ENC/local-src-$v" + done + + # --- keychain (macOS only) -------------------------------------- + if [[ "$OSTYPE" == darwin* ]]; then + ENC_KC_SVC="secretenv-enc-$$" + for v in $ENC_VALUES; do + enc_seed "$ENC/$v.expected" security add-generic-password -U \ + -s "$ENC_KC_SVC" -a "$v" -w __VAL__ >/dev/null 2>&1 + # -g declares the encoding explicitly; decode when it says hex. + security find-generic-password -s "$ENC_KC_SVC" -a "$v" -g 2>&1 >/dev/null \ + | perl -ne 'if(/^password: 0x([0-9A-Fa-f]+)/){print pack("H*",$1); exit} + elsif(/^password: "(.*)"$/s){print $1; exit}' \ + > "$ENC/keychain-$v.raw" + assert_native_holds "3703-$v keychain native holds bytes" \ + "$ENC/$v.expected" "$ENC/keychain-$v.raw" "no" + done + ENC_CFG=$(enc_mkenv keychain '[backends.keychain-enc] +type = "keychain"' "keychain-enc://$ENC_KC_SVC/%V%") + for v in $ENC_VALUES; do + assert_value_bytes "3704-$v keychain round-trip" \ + "$ENC/$v.expected" "$ENC_CFG" "$v" "$ENC/keychain-$v.log" "$ENC/keychain-$v.raw" + done + for v in $ENC_VALUES; do + security delete-generic-password -s "$ENC_KC_SVC" -a "$v" >/dev/null 2>&1 + done + else + record "3703 keychain encoding round-trip" "SKIP" "host is $OSTYPE" + fi + + # --- vault ------------------------------------------------------ + for v in $ENC_VALUES; do + enc_seed "$ENC/$v.expected" vault kv put "secret/encoding/$v" value=__VAL__ >/dev/null 2>&1 + vault kv get -field=value "secret/encoding/$v" > "$ENC/vault-$v.raw" 2>/dev/null + assert_native_holds "3705-$v vault native holds bytes" \ + "$ENC/$v.expected" "$ENC/vault-$v.raw" "no" + done + ENC_CFG=$(enc_mkenv vault "[backends.vault-dev] +type = \"vault\" +vault_address = \"${VAULT_ADDR:-http://127.0.0.1:8200}\"" 'vault-dev:///secret/encoding/%V%') + for v in $ENC_VALUES; do + assert_value_bytes "3706-$v vault round-trip" \ + "$ENC/$v.expected" "$ENC_CFG" "$v" "$ENC/vault-$v.log" "$ENC/vault-$v.raw" + done + + # --- openbao ---------------------------------------------------- + ENC_BAO_ADDR="${SECRETENV_TEST_BAO_ADDR:-http://127.0.0.1:8300}" + for v in $ENC_VALUES; do + BAO_ADDR="$ENC_BAO_ADDR" enc_seed "$ENC/$v.expected" \ + bao kv put "secret/encoding/$v" value=__VAL__ >/dev/null 2>&1 + BAO_ADDR="$ENC_BAO_ADDR" bao kv get -field=value "secret/encoding/$v" \ + > "$ENC/openbao-$v.raw" 2>/dev/null + assert_native_holds "3707-$v openbao native holds bytes" \ + "$ENC/$v.expected" "$ENC/openbao-$v.raw" "no" + done + ENC_CFG=$(enc_mkenv openbao "[backends.openbao-dev] +type = \"openbao\" +bao_address = \"$ENC_BAO_ADDR\"" 'openbao-dev:///secret/encoding/%V%') + for v in $ENC_VALUES; do + assert_value_bytes "3708-$v openbao round-trip" \ + "$ENC/$v.expected" "$ENC_CFG" "$v" "$ENC/openbao-$v.log" "$ENC/openbao-$v.raw" + done + + # --- aws-ssm ---------------------------------------------------- + for v in $ENC_VALUES; do + aws ssm put-parameter --name "/secretenv-encoding/$v" --type SecureString \ + --value "file://$ENC/$v.expected" --overwrite --region "$AWS_REGION" >/dev/null 2>&1 + aws ssm get-parameter --name "/secretenv-encoding/$v" --with-decryption \ + --query Parameter.Value --output text --region "$AWS_REGION" \ + > "$ENC/aws-ssm-$v.raw" 2>/dev/null + assert_native_holds "3709-$v aws-ssm native holds bytes" \ + "$ENC/$v.expected" "$ENC/aws-ssm-$v.raw" "yes" + done + ENC_CFG=$(enc_mkenv awsssm "[backends.aws-ssm-prod] +type = \"aws-ssm\" +aws_region = \"$AWS_REGION\"" 'aws-ssm-prod:///secretenv-encoding/%V%') + for v in $ENC_VALUES; do + assert_value_bytes "3710-$v aws-ssm round-trip" \ + "$ENC/$v.expected" "$ENC_CFG" "$v" "$ENC/aws-ssm-$v.log" "$ENC/aws-ssm-$v.raw" + done + + # --- aws-secrets ------------------------------------------------ + for v in $ENC_VALUES; do + aws secretsmanager create-secret --name "secretenv-encoding-$v" \ + --secret-string "file://$ENC/$v.expected" --region "$AWS_REGION" >/dev/null 2>&1 \ + || aws secretsmanager put-secret-value --secret-id "secretenv-encoding-$v" \ + --secret-string "file://$ENC/$v.expected" --region "$AWS_REGION" >/dev/null 2>&1 + aws secretsmanager get-secret-value --secret-id "secretenv-encoding-$v" \ + --query SecretString --output text --region "$AWS_REGION" \ + > "$ENC/aws-secrets-$v.raw" 2>/dev/null + assert_native_holds "3711-$v aws-secrets native holds bytes" \ + "$ENC/$v.expected" "$ENC/aws-secrets-$v.raw" "yes" + done + ENC_CFG=$(enc_mkenv awssec "[backends.aws-secrets-prod] +type = \"aws-secrets\" +aws_region = \"$AWS_REGION\"" 'aws-secrets-prod:///secretenv-encoding-%V%') + for v in $ENC_VALUES; do + assert_value_bytes "3712-$v aws-secrets round-trip" \ + "$ENC/$v.expected" "$ENC_CFG" "$v" "$ENC/aws-secrets-$v.log" "$ENC/aws-secrets-$v.raw" + done + + # --- gcp -------------------------------------------------------- + for v in $ENC_VALUES; do + gcloud secrets create "secretenv_encoding_$v" --replication-policy=automatic \ + --project "$GCP_PROJECT" >/dev/null 2>&1 + gcloud secrets versions add "secretenv_encoding_$v" \ + --data-file="$ENC/$v.expected" --project "$GCP_PROJECT" >/dev/null 2>&1 + # `versions access` writes the payload raw, with NO added newline. + gcloud secrets versions access latest --secret "secretenv_encoding_$v" \ + --project "$GCP_PROJECT" > "$ENC/gcp-$v.raw" 2>/dev/null + assert_native_holds "3713-$v gcp native holds bytes" \ + "$ENC/$v.expected" "$ENC/gcp-$v.raw" "no" + done + ENC_CFG=$(enc_mkenv gcp "[backends.gcp-prod] +type = \"gcp\" +gcp_project = \"$GCP_PROJECT\"" 'gcp-prod:///secretenv_encoding_%V%') + for v in $ENC_VALUES; do + assert_value_bytes "3714-$v gcp round-trip" \ + "$ENC/$v.expected" "$ENC_CFG" "$v" "$ENC/gcp-$v.log" "$ENC/gcp-$v.raw" + done + + # --- azure ------------------------------------------------------ + for v in $ENC_VALUES; do + az keyvault secret set --vault-name "$AZURE_VAULT" \ + --name "secretenv-encoding-$v" --file "$ENC/$v.expected" >/dev/null 2>&1 + az keyvault secret show --vault-name "$AZURE_VAULT" \ + --name "secretenv-encoding-$v" --query value -o tsv \ + > "$ENC/azure-$v.raw" 2>/dev/null + assert_native_holds "3715-$v azure native holds bytes" \ + "$ENC/$v.expected" "$ENC/azure-$v.raw" "yes" + done + ENC_CFG=$(enc_mkenv azure "[backends.azure-prod] +type = \"azure\" +azure_vault_url = \"https://$AZURE_VAULT.vault.azure.net/\"" 'azure-prod:///secretenv-encoding-%V%') + for v in $ENC_VALUES; do + assert_value_bytes "3716-$v azure round-trip" \ + "$ENC/$v.expected" "$ENC_CFG" "$v" "$ENC/azure-$v.log" "$ENC/azure-$v.raw" + done + + # --- doppler ---------------------------------------------------- + for v in $ENC_VALUES; do + ENC_UV=$(printf '%s' "$v" | tr 'a-z' 'A-Z') + enc_seed "$ENC/$v.expected" doppler secrets set "ENCODING_$ENC_UV" __VAL__ \ + --project secretenv-validation --config dev --silent >/dev/null 2>&1 + doppler secrets get "ENCODING_$ENC_UV" --plain \ + --project secretenv-validation --config dev > "$ENC/doppler-$v.raw" 2>/dev/null + assert_native_holds "3717-$v doppler native holds bytes" \ + "$ENC/$v.expected" "$ENC/doppler-$v.raw" "yes" + done + ENC_CFG=$(enc_mkenv doppler '[backends.doppler-test] +type = "doppler"' 'doppler-test:///secretenv-validation/dev/ENCODING_%U%') + for v in $ENC_VALUES; do + assert_value_bytes "3718-$v doppler round-trip" \ + "$ENC/$v.expected" "$ENC_CFG" "$v" "$ENC/doppler-$v.log" "$ENC/doppler-$v.raw" + done + # MUST clean up: Doppler branch configs inherit from their root + # config, so an ENCODING_* key left in `dev` also shows up in + # `dev_registry`, where `registry list` then tries to parse a + # non-URI value and fails (self-inflicted break of assertions + # 312-314 on the following run). + for v in $ENC_VALUES; do + ENC_UV=$(printf '%s' "$v" | tr 'a-z' 'A-Z') + doppler secrets delete "ENCODING_$ENC_UV" --project secretenv-validation \ + --config dev --yes >/dev/null 2>&1 + done + + # --- infisical -------------------------------------------------- + ENC_IFP="${SECRETENV_INFISICAL_PROJECT_ID:-46302876-3c2f-4349-9376-f8a8228bdb1e}" + for v in $ENC_VALUES; do + ENC_UV=$(printf '%s' "$v" | tr 'a-z' 'A-Z') + enc_seed "$ENC/$v.expected" infisical secrets set "ENCODING_$ENC_UV=__VAL__" \ + --projectId "$ENC_IFP" --env dev --path / --type shared >/dev/null 2>&1 + infisical secrets get "ENCODING_$ENC_UV" --plain \ + --projectId "$ENC_IFP" --env dev --path / > "$ENC/infisical-$v.raw" 2>/dev/null + assert_native_holds "3719-$v infisical native holds bytes" \ + "$ENC/$v.expected" "$ENC/infisical-$v.raw" "yes" + done + ENC_CFG=$(enc_mkenv infisical '[backends.infisical-test] +type = "infisical"' "infisical-test:///$ENC_IFP/dev/ENCODING_%U%") + for v in $ENC_VALUES; do + assert_value_bytes "3720-$v infisical round-trip" \ + "$ENC/$v.expected" "$ENC_CFG" "$v" "$ENC/infisical-$v.log" "$ENC/infisical-$v.raw" + done + # Same flat-namespace hazard as Doppler above. + for v in $ENC_VALUES; do + ENC_UV=$(printf '%s' "$v" | tr 'a-z' 'A-Z') + # `--type shared` is required: it defaults to `personal`, and a + # mismatched type deletes nothing while still exiting 0. + infisical secrets delete "ENCODING_$ENC_UV" --projectId "$ENC_IFP" \ + --env dev --path / --type shared >/dev/null 2>&1 + done + + # --- cf-kv ------------------------------------------------------- + # NOT a corrupting backend, and this block exists to keep it that + # way. `wrangler kv key get --text` DOES append a trailing newline, + # so cf-kv's strip of exactly one is correct. An ad-hoc probe that + # omitted `--text` briefly suggested otherwise and nearly shipped a + # regression here — hence a permanent round-trip guard using the + # backend's real argv rather than a hand-written approximation. + for v in $ENC_VALUES; do + ENC_UV=$(printf '%s' "$v" | tr 'a-z' 'A-Z') + wrangler kv key put --namespace-id "$CFKV_NS" --remote \ + --path "$ENC/$v.expected" "ENCODING_$ENC_UV" /dev/null 2>&1 + wrangler kv key get --namespace-id "$CFKV_NS" --remote --text "ENCODING_$ENC_UV" \ + > "$ENC/cf-kv-$v.raw" 2>/dev/null + assert_native_holds "3721-$v cf-kv native holds bytes" \ + "$ENC/$v.expected" "$ENC/cf-kv-$v.raw" "yes" + done + ENC_CFG=$(enc_mkenv cfkv '[backends.cf-kv-test] +type = "cf-kv"' "cf-kv-test:///$CFKV_NS/ENCODING_%U%") + for v in $ENC_VALUES; do + assert_value_bytes "3722-$v cf-kv round-trip" \ + "$ENC/$v.expected" "$ENC_CFG" "$v" "$ENC/cf-kv-$v.log" "$ENC/cf-kv-$v.raw" + done + for v in $ENC_VALUES; do + ENC_UV=$(printf '%s' "$v" | tr 'a-z' 'A-Z') + wrangler kv key delete --namespace-id "$CFKV_NS" --remote "ENCODING_$ENC_UV" \ + /dev/null 2>&1 + done + + # --- 1password --------------------------------------------------- + for v in $ENC_VALUES; do + op item delete "secretenv-encoding-$v" --vault Private >/dev/null 2>&1 + enc_seed "$ENC/$v.expected" op item create --category Password \ + --title "secretenv-encoding-$v" --vault Private password=__VAL__ >/dev/null 2>&1 + op read "op://Private/secretenv-encoding-$v/password" \ + > "$ENC/1password-$v.raw" 2>/dev/null + assert_native_holds "3723-$v 1password native holds bytes" \ + "$ENC/$v.expected" "$ENC/1password-$v.raw" "yes" + done + ENC_CFG=$(enc_mkenv onepassword '[backends.1password-private] +type = "1password"' '1password-private://Private/secretenv-encoding-%V%/password') + for v in $ENC_VALUES; do + assert_value_bytes "3724-$v 1password round-trip" \ + "$ENC/$v.expected" "$ENC_CFG" "$v" "$ENC/1password-$v.log" "$ENC/1password-$v.raw" + done + for v in $ENC_VALUES; do + op item delete "secretenv-encoding-$v" --vault Private >/dev/null 2>&1 + done + + # --- conjur ------------------------------------------------------ + # The smoke policy defines exactly four variables and adding one + # needs a policy load, so this reuses `secretenv-smoke/cycle` (the + # designated scratch variable) one value at a time and restores it + # to `{}` afterwards. + ENC_CJ_URL="${SECRETENV_TEST_CONJUR_URL:-http://localhost:8083}" + ENC_CJ_ACCT="${SECRETENV_TEST_CONJUR_ACCOUNT:-myorg}" + ENC_CJ_CFG="$ENC/conjur-config.toml" + for v in $ENC_VALUES; do + # The conjur CLI is a Docker wrapper: a host path passed to -f + # does not exist inside the container. Piping into -f /dev/stdin + # is the form that works (and preserves exact bytes). + <"$ENC/$v.expected" CONJUR_APPLIANCE_URL="$ENC_CJ_URL" CONJUR_ACCOUNT="$ENC_CJ_ACCT" \ + conjur variable set -i secretenv-smoke/cycle -f /dev/stdin >/dev/null 2>&1 + CONJUR_APPLIANCE_URL="$ENC_CJ_URL" CONJUR_ACCOUNT="$ENC_CJ_ACCT" \ + conjur variable get -i secretenv-smoke/cycle > "$ENC/conjur-$v.raw" 2>/dev/null + assert_native_holds "3725-$v conjur native holds bytes" \ + "$ENC/$v.expected" "$ENC/conjur-$v.raw" "yes" + printf 'enc = "conjur-dev:///secretenv-smoke/cycle"\n' > "$ENC/conjur-registry.toml" + { + printf '[registries.default]\nsources = ["local-main://%s"]\n\n' \ + "$ENC/conjur-registry.toml" + printf '[backends.local-main]\ntype = "local"\n\n' + printf '[backends.conjur-dev]\ntype = "conjur"\nconjur_url = "%s"\nconjur_account = "%s"\n' \ + "$ENC_CJ_URL" "$ENC_CJ_ACCT" + } > "$ENC_CJ_CFG" + assert_value_bytes "3726-$v conjur round-trip" \ + "$ENC/$v.expected" "$ENC_CJ_CFG" "enc" "$ENC/conjur-$v.log" "$ENC/conjur-$v.raw" + done + printf '{}' | CONJUR_APPLIANCE_URL="$ENC_CJ_URL" CONJUR_ACCOUNT="$ENC_CJ_ACCT" \ + conjur variable set -i secretenv-smoke/cycle -f /dev/stdin >/dev/null 2>&1 + + # --- bitwarden-sm ------------------------------------------------ + # Addressed by UUID, so the registry is built from the id the CLI + # returns at create time rather than from a URI template. + ENC_BWS_PROJECT=$(bws project list -o json 2>/dev/null \ + | perl -ne 'if(/"id"\s*:\s*"([0-9a-f-]{36})"/){print $1; exit}') + if [ -n "$ENC_BWS_PROJECT" ]; then + : > "$ENC/bws-registry.toml" + ENC_BWS_IDS="" + for v in $ENC_VALUES; do + ENC_UV=$(printf '%s' "$v" | tr 'a-z' 'A-Z') + ENC_BWS_STALE=$(bws secret list -o json 2>/dev/null \ + | perl -0777 -ne 'while(/"id"\s*:\s*"([0-9a-f-]{36})".*?"key"\s*:\s*"([^"]+)"/gs){print "$1 $2\n"}' \ + | awk -v k="ENCODING_$ENC_UV" '$2==k {print $1}') + for sid in $ENC_BWS_STALE; do bws secret delete "$sid" >/dev/null 2>&1; done + # `--` is load-bearing: a PEM value starts with '-', which + # bws's arg parser otherwise reads as a flag and rejects. + ENC_BWS_ID=$(enc_seed "$ENC/$v.expected" bws secret create -- \ + "ENCODING_$ENC_UV" __VAL__ "$ENC_BWS_PROJECT" 2>/dev/null \ + | perl -ne 'if(/"id"\s*:\s*"([0-9a-f-]{36})"/){print $1; exit}') + if [ -z "$ENC_BWS_ID" ]; then + record "3727-$v bitwarden-sm native holds bytes" "SKIP" "bws secret create failed" + continue + fi + ENC_BWS_IDS="$ENC_BWS_IDS $ENC_BWS_ID" + bws secret get "$ENC_BWS_ID" -o json 2>/dev/null \ + | perl -0777 -ne 'if(/"value"\s*:\s*"((?:[^"\\]|\\.)*)"/){my $x=$1; $x=~s/\\n/\n/g; $x=~s/\\t/\t/g; $x=~s/\\"/"/g; $x=~s/\\\\/\\/g; print $x}' \ + > "$ENC/bitwarden-sm-$v.raw" + assert_native_holds "3727-$v bitwarden-sm native holds bytes" \ + "$ENC/$v.expected" "$ENC/bitwarden-sm-$v.raw" "no" + printf '%s = "bws-dev://%s"\n' "$v" "$ENC_BWS_ID" >> "$ENC/bws-registry.toml" + done + { + printf '[registries.default]\nsources = ["local-main://%s"]\n\n' "$ENC/bws-registry.toml" + printf '[backends.local-main]\ntype = "local"\n\n' + printf '[backends.bws-dev]\ntype = "bitwarden-sm"\n' + } > "$ENC/bws-config.toml" + for v in $ENC_VALUES; do + if grep -q "^$v = " "$ENC/bws-registry.toml" 2>/dev/null; then + assert_value_bytes "3728-$v bitwarden-sm round-trip" \ + "$ENC/$v.expected" "$ENC/bws-config.toml" "$v" \ + "$ENC/bitwarden-sm-$v.log" "$ENC/bitwarden-sm-$v.raw" + fi + done + for id in $ENC_BWS_IDS; do bws secret delete "$id" >/dev/null 2>&1; done + else + record "3727 bitwarden-sm encoding round-trip" "SKIP" "no bws project id resolvable" + fi + + # --- keeper ------------------------------------------------------ + # Deliberately not seeded. Keeper record creation needs a record + # type + folder and would mutate the operator's vault; the two read + # paths were instead verified directly: `--format=json` extracts the + # field from parsed JSON and applies no strip, and + # `keeper get --format=password` was measured live to append a + # trailing newline, so its strip of exactly one is correct. Recorded + # as an explicit SKIP so the coverage gap stays visible in the + # matrix rather than silently reading as "covered". + record "3729 keeper encoding round-trip" "SKIP" \ + "not seeded (record-add would mutate the operator vault); both read paths verified by inspection + live newline probe" +fi + + # --------------------------------------------------------------- # Summary # ---------------------------------------------------------------