Fix pass Git config and lazy OTP commits

This commit is contained in:
Hermes Agent
2026-08-10 09:42:53 +00:00
parent 6ea184cbc1
commit c86ea9eb0e
8 changed files with 231 additions and 22 deletions

View File

@@ -832,13 +832,7 @@ fn execute_otp<B: SecretStoreBackend, P: CliPresentation, I: OtpInteraction, O:
Ok(timestamp) => timestamp,
Err(error) => return operation_error(stderr, error),
};
let mut committer =
match automatic_committer(&repository, request.entry.trim_end_matches('/')) {
Ok(committer) => committer,
Err(error) => return operation_error(stderr, error),
};
let outcome =
match service.code(&request.entry, timestamp, None, secrets, &mut committer) {
let outcome = match service.code_automatic(&request.entry, timestamp, None, secrets) {
Ok(outcome) => outcome,
Err(error) => return operation_error(stderr, error),
};
@@ -1488,6 +1482,7 @@ mod tests {
error::Error,
ffi::OsString,
fs,
io::Write as _,
sync::{Arc, Mutex},
};
@@ -1502,7 +1497,7 @@ mod tests {
},
config::Config,
crypto::KeyInfo,
git::GitCredentialProvider as _,
git::{GitCredentialProvider as _, GitIdentity, GitRepository},
otp::OtpInput,
presentation::{ClipboardTimeout, QrMatrix},
repository::{EntryPath, Repository, SecretBytes},
@@ -2097,12 +2092,28 @@ mod tests {
let fixtures = std::path::Path::new(env!("CARGO_MANIFEST_DIR"))
.join("../../crates/storage/tests/fixtures/compatibility");
let temporary = tempfile::tempdir()?;
let vault = temporary.path().join("vault");
fs::create_dir_all(vault.join("otp"))?;
for path in [".gpg-id", ".gpg-id.sig", "otp/totp.gpg"] {
fs::copy(fixtures.join("stores/basic").join(path), vault.join(path))?;
}
let repository = Repository::open(&vault)?;
let identity = GitIdentity::ironstorage();
let git = GitRepository::init(&repository, identity.clone())?;
let commits_before = git.log(None)?.len();
drop(git);
let git_config = vault.join(".git/config");
let mut config_file = fs::OpenOptions::new().append(true).open(&git_config)?;
config_file.write_all(&fs::read(fixtures.join("pass-git-config.ini"))?)?;
drop(config_file);
let config_before = fs::read(&git_config)?;
let ciphertext_before = fs::read(vault.join("otp/totp.gpg"))?;
let config_path = temporary.path().join("config.toml");
fs::write(
&config_path,
format!(
"vault = {:?}\ndefault_key = {:?}\nkey_material = {:?}\nclipboard_timeout_seconds = 1\n",
fixtures.join("stores/basic"),
vault,
FINGERPRINT,
fixtures.join("keys"),
),
@@ -2114,6 +2125,29 @@ mod tests {
let mut stdout = Vec::new();
let mut stderr = Vec::new();
assert_eq!(
execute_secure_with_services(
&config,
&CommandRequest::Show(ShowRequest {
entry: Some("otp/totp".to_owned()),
presentation: Presentation::Terminal,
}),
&mut secrets,
&mut presentation,
&mut interaction,
|| Ok(59),
&mut stdout,
&mut stderr,
)
.expect("memory output cannot fail"),
EXIT_SUCCESS
);
assert!(
stdout
.windows(b"otpauth://totp".len())
.any(|part| part == b"otpauth://totp")
);
stdout.clear();
assert_eq!(
execute_secure_with_services(
&config,
@@ -2136,6 +2170,10 @@ mod tests {
assert!(code.iter().all(u8::is_ascii_digit));
assert!(!stdout.windows(code.len()).any(|part| part == code));
assert!(stderr.is_empty());
assert_eq!(fs::read(vault.join("otp/totp.gpg"))?, ciphertext_before);
assert_eq!(fs::read(&git_config)?, config_before);
let git = GitRepository::open(&repository, identity)?;
assert_eq!(git.log(None)?.len(), commits_before);
stdout.clear();
let uri = fs::read(fixtures.join("expected/basic/otp/totp.txt"))?;

View File

@@ -2476,20 +2476,16 @@ fn validate_local_config_security(config: &gix_config::File) -> Result<(), GitEr
let name = section.header().name().to_str_lossy().to_ascii_lowercase();
if matches!(
name.as_str(),
"include"
| "includeif"
| "credential"
| "filter"
| "diff"
| "merge"
| "protocol"
| "url"
"include" | "includeif" | "credential" | "filter" | "merge" | "protocol" | "url"
) {
return Err(GitError::InvalidRepository(format!(
"unsafe Git configuration section: {name}"
)));
}
}
// Upstream `pass git init` installs diff.gpg.binary and
// diff.gpg.textconv. IronStorage parses blobs and renders diffs itself, so
// these passive settings are preserved but never evaluated or executed.
for key in [
"core.hooksPath",
"core.sshCommand",

View File

@@ -11,6 +11,7 @@ use zeroize::Zeroize as _;
use crate::{
command::{OtpAppendRequest, OtpInputSource, OtpInsertRequest},
crypto::{CryptoError, KeyStore, SecretProvider},
git::{AutomaticEntryCommitter, GitError, GitIdentity},
recipient::{RecipientPolicyError, RecipientPolicyManager, SigningPolicy},
repository::{EncryptedEntry, EntryPath, Repository, RepositoryError, SecretBytes},
write::{EntryAction, EntryCommit, EntryCommitError, EntryCommitter, OverwriteDecision},
@@ -613,12 +614,80 @@ impl<'a> OtpService<'a> {
provider: &mut impl SecretProvider,
committer: &mut impl EntryCommitter,
) -> Result<OtpCodeOutcome, OtpError> {
let (path, original, plaintext, range, uri) = self.load_code_entry(entry, provider)?;
self.finish_code(
path,
original,
plaintext,
range,
uri,
unix_seconds,
signing,
committer,
)
}
/// Generate a code with storage-owned lazy Git selection. TOTP is
/// read-only and never opens Git; HOTP opens the innermost repository only
/// after the token has been decrypted and identified as counter based.
pub fn code_automatic(
&self,
entry: &str,
unix_seconds: u64,
signing: Option<&SigningPolicy>,
provider: &mut impl SecretProvider,
) -> Result<OtpCodeOutcome, OtpError> {
let (path, original, plaintext, range, uri) = self.load_code_entry(entry, provider)?;
if uri.kind() == OtpKind::Totp {
return Ok(OtpCodeOutcome {
code: uri.code_at(unix_seconds)?,
counter: None,
});
}
let entry = path.to_string();
let mut committer = AutomaticEntryCommitter::for_entry(
self.repository,
&entry,
GitIdentity::ironstorage(),
)?;
self.finish_code(
path,
original,
plaintext,
range,
uri,
unix_seconds,
signing,
&mut committer,
)
}
fn load_code_entry(
&self,
entry: &str,
provider: &mut impl SecretProvider,
) -> Result<(EntryPath, EncryptedEntry, SecretBytes, Range<usize>, OtpUri), OtpError> {
let path = parse_entry(entry)?;
let original = self.repository.read_entry(&path)?;
let plaintext = self.keys.decrypt(&original, provider)?;
let (range, uri) = find_uri(&plaintext, &path)?.ok_or_else(|| OtpError::MissingUri {
entry: path.clone(),
})?;
Ok((path, original, plaintext, range, uri))
}
#[allow(clippy::too_many_arguments)]
fn finish_code(
&self,
path: EntryPath,
original: EncryptedEntry,
plaintext: SecretBytes,
range: Range<usize>,
uri: OtpUri,
unix_seconds: u64,
signing: Option<&SigningPolicy>,
committer: &mut impl EntryCommitter,
) -> Result<OtpCodeOutcome, OtpError> {
match uri.kind() {
OtpKind::Totp => Ok(OtpCodeOutcome {
code: uri.code_at(unix_seconds)?,
@@ -722,6 +791,7 @@ pub enum OtpError {
Repository(RepositoryError),
Crypto(CryptoError),
RecipientPolicy(RecipientPolicyError),
Git(GitError),
InvalidUri,
InvalidScheme,
UnsupportedType,
@@ -771,6 +841,7 @@ impl fmt::Display for OtpError {
Self::Repository(error) => error.fmt(formatter),
Self::Crypto(error) => error.fmt(formatter),
Self::RecipientPolicy(error) => error.fmt(formatter),
Self::Git(error) => error.fmt(formatter),
Self::InvalidUri => formatter.write_str("OTP key URI is not valid UTF-8 URI text"),
Self::InvalidScheme => formatter.write_str("OTP key URI must use the otpauth scheme"),
Self::UnsupportedType => formatter.write_str("OTP key URI type must be totp or hotp"),
@@ -848,6 +919,12 @@ impl From<RecipientPolicyError> for OtpError {
}
}
impl From<GitError> for OtpError {
fn from(error: GitError) -> Self {
Self::Git(error)
}
}
fn validate_input(input: &[u8]) -> Result<(), OtpError> {
if input.is_empty() {
Err(OtpError::EmptyInput)

View File

@@ -30,6 +30,11 @@ password-store tree, and the expected plaintext. It deliberately lives outside
the reproducible generator's `stores/`, `expected/`, and `repositories/`
directories so a refresh cannot silently replace genuine GnuPG producer output.
`pass-git-config.ini` records the local diff driver written by upstream
`pass git init`. IronStorage preserves this configuration for repository
compatibility, but its embedded Rust diff implementation never evaluates the
driver or launches the configured GnuPG command.
Rust tests parse keys, verify signatures, decrypt every entry, compare exact
plaintext bytes, validate ciphertext digests, validate Git objects and refs,
and materialize isolated stores and repositories. They never invoke `pass`,

View File

@@ -0,0 +1,3 @@
[diff "gpg"]
binary = true
textconv = gpg -d --no-tty --batch --quiet --yes

View File

@@ -134,6 +134,10 @@ fn remotes_and_config_are_local_https_only() -> TestResult {
git.config_set("user.name", "Local User")?;
assert_eq!(git.config_get("user.name")?.as_deref(), Some("Local User"));
assert!(git.config_set("credential.helper", "evil").is_err());
assert!(
git.config_set("diff.gpg.textconv", "external-helper")
.is_err()
);
git.remove_remote("origin")?;
assert!(git.remotes().is_empty());
Ok(())

View File

@@ -2,12 +2,13 @@
mod support;
use std::collections::BTreeMap;
use std::{collections::BTreeMap, fs, io::Write as _};
use data_encoding::BASE32_NOPAD;
use ironstorage::{
command::{OtpAppendRequest, OtpInputSource, OtpInsertRequest},
crypto::{KeyInfo, KeyStore, SecretProvider, SecretProviderError},
git::{GitIdentity, GitRepository},
otp::{OtpAlgorithm, OtpError, OtpInput, OtpKind, OtpService, OtpUri},
recipient::RecipientPolicyManager,
repository::{EntryPath, Repository, SecretBytes},
@@ -454,6 +455,84 @@ fn malformed_ambiguous_declined_and_commit_failures_never_mutate() -> TestResult
Ok(())
}
#[test]
fn automatic_code_supports_pass_diff_config_and_opens_git_only_for_hotp() -> TestResult {
let fixture = FixtureSet::load()?;
let store = fixture.materialize_store("basic")?;
let repository = Repository::open(store.path())?;
let identity = GitIdentity::ironstorage();
let git = GitRepository::init(&repository, identity.clone())?;
let initial_commits = git.log(None)?.len();
drop(git);
let config_path = store.path().join(".git/config");
let mut config = fs::OpenOptions::new().append(true).open(&config_path)?;
config.write_all(&fixture.read("pass-git-config.ini")?)?;
drop(config);
let original_config = fs::read(&config_path)?;
let keys = KeyStore::load(fixture.path("keys"))?;
let service = OtpService::new(&repository, &keys);
let mut provider = FixtureSecrets::all(&fixture);
let totp_path = EntryPath::parse("otp/totp")?;
let totp_before = repository.read_entry(&totp_path)?;
let totp = service.code_automatic("otp/totp", 59, None, &mut provider)?;
assert_eq!(totp.counter(), None);
assert_eq!(repository.read_entry(&totp_path)?, totp_before);
let git = GitRepository::open(&repository, identity.clone())?;
assert_eq!(git.log(None)?.len(), initial_commits);
drop(git);
let hotp = service.code_automatic("otp/hotp", 0, None, &mut provider)?;
assert_eq!(hotp.counter(), Some(1));
assert_eq!(service.uri("otp/hotp", &mut provider)?.counter(), Some(1));
let git = GitRepository::open(&repository, identity)?;
assert_eq!(git.log(None)?.len(), initial_commits + 1);
assert_eq!(
git.log(Some(1))?[0].message(),
"Increment HOTP counter for otp/hotp."
);
assert_eq!(fs::read(config_path)?, original_config);
Ok(())
}
#[test]
fn read_only_totp_ignores_git_that_mutations_correctly_reject() -> TestResult {
let fixture = FixtureSet::load()?;
let store = fixture.materialize_store("basic")?;
let repository = Repository::open(store.path())?;
GitRepository::init(&repository, GitIdentity::ironstorage())?;
let config_path = store.path().join(".git/config");
let mut config = fs::OpenOptions::new().append(true).open(&config_path)?;
config.write_all(b"[filter \"unsafe\"]\n\tclean = external-helper\n")?;
drop(config);
let keys = KeyStore::load(fixture.path("keys"))?;
let service = OtpService::new(&repository, &keys);
let mut provider = FixtureSecrets::all(&fixture);
let totp_path = EntryPath::parse("otp/totp")?;
let totp_before = repository.read_entry(&totp_path)?;
assert!(
service
.code_automatic("otp/totp", 59, None, &mut provider)
.is_ok()
);
assert_eq!(repository.read_entry(&totp_path)?, totp_before);
let hotp_path = EntryPath::parse("otp/hotp")?;
let hotp_before = repository.read_entry(&hotp_path)?;
let head_before = fs::read(store.path().join(".git/refs/heads/main"))?;
assert!(matches!(
service.code_automatic("otp/hotp", 0, None, &mut provider),
Err(OtpError::Git(_))
));
assert_eq!(repository.read_entry(&hotp_path)?, hotp_before);
assert_eq!(
fs::read(store.path().join(".git/refs/heads/main"))?,
head_before
);
Ok(())
}
fn token(secret: &[u8], algorithm: &str) -> Result<OtpUri, OtpError> {
OtpUri::parse_str(&format!(
"otpauth://totp/RFC6238?secret={}&algorithm={algorithm}&digits=8&period=30",

View File

@@ -3,7 +3,14 @@
All Git behavior is implemented in `crates/storage`. IronStorage never launches
`git`, a credential helper, an SSH client, a hook, a filter, or a merge driver.
Repositories are opened with isolated configuration and environment access;
repository-local configuration that could name an executable is rejected.
repository-local configuration that can affect an operation is rejected.
Upstream `pass git init` deliberately writes `diff.gpg.binary` and a
`diff.gpg.textconv` GnuPG command. IronStorage preserves these passive keys for
compatibility but never evaluates them: it reads Git blobs and renders decrypted
diffs through Rust storage APIs. Frontends cannot add or change diff drivers.
This behavior follows the
[upstream password-store initialization](https://git.zx2c4.com/password-store/tree/src/password-store.sh)
while keeping runtime helper execution disabled.
`GitRepository` initializes and opens password-store worktrees, selects the
innermost repository for a nested entry, and implements status, log, diff, add,