From b18557aef2a87ccad74eb45c284691d6e08dd4aa Mon Sep 17 00:00:00 2001 From: Jun Cho Date: Sun, 30 Aug 2026 13:40:08 +0900 Subject: [PATCH] fix: honor updateRef when creating signed commits --- src/commit.rs | 93 +++++++++++++++++++- tests/commit.spec.ts | 198 +++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 282 insertions(+), 9 deletions(-) diff --git a/src/commit.rs b/src/commit.rs index 184d9df..b31a7fc 100644 --- a/src/commit.rs +++ b/src/commit.rs @@ -434,19 +434,50 @@ impl Repository { .ok_or(crate::Error::SignatureNotFound)?; let oid = if let Some(signature_str) = signature { + // `commit_signed()` only writes the commit object, unlike `commit()` which + // also validates parents against the ref tip and moves the ref inside + // libgit2. Mirror that behavior here so both paths behave the same. + let update_target = update_ref + .as_deref() + .map(|name| self.resolve_commit_update_ref(name)) + .transpose()?; + + let parents = parents.unwrap_or_default(); + if let Some(CommitUpdateRef::Resolved(reference)) = &update_target { + // Validate before writing the object so a failure leaves no orphaned + // commit in the odb, matching libgit2's ordering. + let first_parent = parents.first().map(|parent| parent.id()); + if reference.target() != first_parent { + return Err( + git2::Error::new( + git2::ErrorCode::Modified, + git2::ErrorClass::Object, + "failed to create commit: current tip is not the first parent", + ) + .into(), + ); + } + } + let commit_content = self.inner.commit_create_buffer( &author, &committer, &message, &tree.inner, - &parents.unwrap_or_default().iter().collect::>(), + &parents.iter().collect::>(), )?; let commit_content_str = std::str::from_utf8(&commit_content)?.to_string(); - self + let oid = self .inner - .commit_signed(&commit_content_str, &signature_str, signature_field.as_deref())? + .commit_signed(&commit_content_str, &signature_str, signature_field.as_deref())?; + + if let Some(update_target) = update_target { + self.update_ref_for_commit(update_target, oid)?; + } + + oid } else { self.inner.commit( update_ref.as_deref(), @@ -461,3 +492,59 @@ impl Repository { Ok(oid.to_string()) } } + +/// Resolution of an `updateRef` name for commit creation, mirroring how libgit2 +/// treats the ref on its unsigned commit path (`git_commit__create_internal`). +enum CommitUpdateRef<'repo> { + /// The name resolved to an existing direct reference. + Resolved(git2::Reference<'repo>), + /// The name (or the branch its symbolic ref points to) does not exist yet; + /// a reference with this name should be created. + Create(String), +} + +impl Repository { + /// Mirrors `git_reference_lookup_resolved` as used by libgit2 when creating + /// a commit: a missing ref or a symbolic ref to an unborn branch (e.g. HEAD + /// in a fresh repository) is not an error, anything else propagates. + fn resolve_commit_update_ref(&self, name: &str) -> crate::Result> { + match self.inner.find_reference(name) { + Ok(reference) => match reference.resolve() { + Ok(resolved) => Ok(CommitUpdateRef::Resolved(resolved)), + Err(e) if e.code() == git2::ErrorCode::NotFound => { + let target = reference.symbolic_target().unwrap_or(name).to_string(); + Ok(CommitUpdateRef::Create(target)) + } + Err(e) => Err(e.into()), + }, + Err(e) if e.code() == git2::ErrorCode::NotFound => Ok(CommitUpdateRef::Create(name.to_string())), + Err(e) => Err(e.into()), + } + } + + /// Points the resolved ref at the new commit, mirroring + /// `git_reference__update_for_commit`: same reflog message format, and + /// updating an existing ref asserts its target has not moved since it was + /// resolved. The reflog identity falls back to the repository default since + /// git2 does not accept a signature on reference updates. + fn update_ref_for_commit(&self, target: CommitUpdateRef<'_>, oid: git2::Oid) -> crate::Result<()> { + let commit = self.inner.find_commit(oid)?; + let commit_type = match commit.parent_count() { + 0 => " (initial)", + 1 => "", + _ => " (merge)", + }; + let reflog_message = format!("commit{}: {}", commit_type, commit.summary().unwrap_or_default()); + match target { + CommitUpdateRef::Resolved(mut reference) => { + reference.set_target(oid, &reflog_message)?; + } + CommitUpdateRef::Create(name) => { + // Create-only (no force), like libgit2's `git_reference__update_terminal`: + // if the ref appeared in the meantime this errors instead of clobbering it. + self.inner.reference(&name, oid, false, &reflog_message)?; + } + } + Ok(()) + } +} diff --git a/tests/commit.spec.ts b/tests/commit.spec.ts index d69bbdd..8523b13 100644 --- a/tests/commit.spec.ts +++ b/tests/commit.spec.ts @@ -1,11 +1,14 @@ import fs from 'node:fs/promises'; import path from 'node:path'; import { describe, expect, it } from 'vitest'; -import { isValidOid, openRepository } from '../index'; +import { initRepository, isValidOid, openRepository } from '../index'; import { useFixture } from './fixtures'; +import { makeTmpDir } from './tmp'; describe('commit', () => { const signature = { name: 'Seokju Na', email: 'seokju.me@gmail.com' }; + const gpgSignature = + '-----BEGIN PGP SIGNATURE-----\\nVersion: GnuPG v1\\n\\niQEcBAABAgAGBQJTest123\\n-----END PGP SIGNATURE-----'; it('get commit', async () => { const p = await useFixture('commits'); @@ -77,18 +80,16 @@ describe('commit', () => { author: signature, committer: signature, parents: [repo.head().target()!], - signature: - '-----BEGIN PGP SIGNATURE-----\\nVersion: GnuPG v1\\n\\niQEcBAABAgAGBQJTest123\\n-----END PGP SIGNATURE-----', + signature: gpgSignature, }); expect(isValidOid(oid)).toBe(true); + expect(repo.head().target()).toEqual(oid); const signatureInfo = repo.extractSignature(oid); expect(signatureInfo).not.toBeNull(); const { signature: extractedSignature = '', signedData = '' } = signatureInfo || {}; - expect(extractedSignature).toEqual( - '-----BEGIN PGP SIGNATURE-----\\nVersion: GnuPG v1\\n\\niQEcBAABAgAGBQJTest123\\n-----END PGP SIGNATURE-----' - ); + expect(extractedSignature).toEqual(gpgSignature); expect(signedData).toContain('tree ab9abf28de846b5968a8f12156f1d5ce3f4a198e'); expect(signedData).toContain('parent a01e9888e46729ef4aa68953ba19b02a7a64eb82'); @@ -97,6 +98,191 @@ describe('commit', () => { expect(signedData).toContain('signed commit'); }); + it('signed commit records reflog entry', async () => { + const p = await useFixture('commits'); + const repo = await openRepository(p); + await fs.writeFile(path.join(p, 'signed'), 'signed'); + const index = repo.index(); + index.addPath('signed'); + const tree = repo.getTree(index.writeTree()); + const oid = repo.commit(tree, 'signed commit\n\nbody the reflog entry must not contain', { + updateRef: 'HEAD', + author: signature, + committer: signature, + parents: [repo.head().target()!], + signature: gpgSignature, + }); + const entry = repo.reflog('HEAD').get(0); + expect(entry?.idNew()).toEqual(oid); + expect(entry?.message()).toEqual('commit: signed commit'); + }); + + it('signed commit updates an existing branch ref', async () => { + const p = await useFixture('commits'); + const repo = await openRepository(p); + const tip = repo.head().target()!; + await fs.writeFile(path.join(p, 'signed'), 'signed'); + const index = repo.index(); + index.addPath('signed'); + const tree = repo.getTree(index.writeTree()); + const oid = repo.commit(tree, 'signed commit on main', { + updateRef: 'refs/heads/main', + author: signature, + committer: signature, + parents: [tip], + signature: gpgSignature, + }); + expect(repo.findReference('refs/heads/main')?.target()).toEqual(oid); + expect(repo.reflog('refs/heads/main').get(0)?.message()).toEqual('commit: signed commit on main'); + }); + + it('create signed commit on unborn HEAD', async () => { + const p = await makeTmpDir('signed-unborn'); + const repo = await initRepository(p, { initialHead: 'main' }); + await fs.writeFile(path.join(p, 'first'), 'first'); + const index = repo.index(); + index.addPath('first'); + const tree = repo.getTree(index.writeTree()); + const oid = repo.commit(tree, 'initial signed commit', { + updateRef: 'HEAD', + author: signature, + committer: signature, + signature: gpgSignature, + }); + expect(repo.head().name()).toEqual('refs/heads/main'); + expect(repo.head().target()).toEqual(oid); + expect(repo.reflog('HEAD').get(0)?.message()).toEqual('commit (initial): initial signed commit'); + }); + + it('create signed commit updating a new ref', async () => { + const p = await useFixture('commits'); + const repo = await openRepository(p); + const headBefore = repo.head().target()!; + await fs.writeFile(path.join(p, 'signed'), 'signed'); + const index = repo.index(); + index.addPath('signed'); + const tree = repo.getTree(index.writeTree()); + const oid = repo.commit(tree, 'signed commit on new branch', { + updateRef: 'refs/heads/sign-target', + author: signature, + committer: signature, + parents: [headBefore], + signature: gpgSignature, + }); + expect(repo.findReference('refs/heads/sign-target')?.target()).toEqual(oid); + expect(repo.head().target()).toEqual(headBefore); + }); + + it('signed commit rejects updateRef when first parent is not the current tip', async () => { + const p = await useFixture('commits'); + const repo = await openRepository(p); + const headBefore = repo.head().target(); + await fs.writeFile(path.join(p, 'signed'), 'signed'); + const index = repo.index(); + index.addPath('signed'); + const tree = repo.getTree(index.writeTree()); + expect(() => + repo.commit(tree, 'orphaned signed commit', { + updateRef: 'HEAD', + author: signature, + committer: signature, + signature: gpgSignature, + }) + ).toThrowError(/current tip is not the first parent/); + const revwalk = repo.revwalk(); + revwalk.pushHead(); + revwalk.next(); + const older = revwalk.next()!; + expect(() => + repo.commit(tree, 'orphaned signed commit', { + updateRef: 'HEAD', + author: signature, + committer: signature, + parents: [older], + signature: gpgSignature, + }) + ).toThrowError(/current tip is not the first parent/); + expect(repo.head().target()).toEqual(headBefore); + }); + + it('signed merge commit records merge reflog message', async () => { + const p = await useFixture('commits'); + const repo = await openRepository(p); + const revwalk = repo.revwalk(); + revwalk.pushHead(); + const tip = revwalk.next()!; + const older = revwalk.next()!; + await fs.writeFile(path.join(p, 'signed'), 'signed'); + const index = repo.index(); + index.addPath('signed'); + const tree = repo.getTree(index.writeTree()); + const oid = repo.commit(tree, 'signed merge commit', { + updateRef: 'HEAD', + author: signature, + committer: signature, + parents: [tip, older], + signature: gpgSignature, + }); + expect(repo.head().target()).toEqual(oid); + expect(repo.reflog('HEAD').get(0)?.message()).toEqual('commit (merge): signed merge commit'); + }); + + it('signed commit skips first-parent validation for a nonexistent ref', async () => { + const p = await useFixture('commits'); + const repo = await openRepository(p); + const headBefore = repo.head().target(); + await fs.writeFile(path.join(p, 'signed'), 'signed'); + const index = repo.index(); + index.addPath('signed'); + const tree = repo.getTree(index.writeTree()); + const oid = repo.commit(tree, 'rootless signed commit', { + updateRef: 'refs/heads/no-validate', + author: signature, + committer: signature, + signature: gpgSignature, + }); + expect(repo.findReference('refs/heads/no-validate')?.target()).toEqual(oid); + expect(repo.head().target()).toEqual(headBefore); + }); + + it('signed commit rejects an invalid updateRef name like unsigned commits', async () => { + const p = await useFixture('commits'); + const repo = await openRepository(p); + await fs.writeFile(path.join(p, 'signed'), 'signed'); + const index = repo.index(); + index.addPath('signed'); + const tree = repo.getTree(index.writeTree()); + expect(() => + repo.commit(tree, 'signed commit', { + updateRef: 'not a valid ref name', + author: signature, + committer: signature, + parents: [repo.head().target()!], + signature: gpgSignature, + }) + ).toThrowError(/not valid/); + }); + + it('signed commit updates detached HEAD', async () => { + const p = await useFixture('commits'); + const repo = await openRepository(p); + const tip = repo.head().target()!; + repo.setHeadDetached(repo.getCommit(tip)); + await fs.writeFile(path.join(p, 'signed'), 'signed'); + const index = repo.index(); + index.addPath('signed'); + const tree = repo.getTree(index.writeTree()); + const oid = repo.commit(tree, 'detached signed commit', { + updateRef: 'HEAD', + author: signature, + committer: signature, + parents: [tip], + signature: gpgSignature, + }); + expect(repo.head().target()).toEqual(oid); + expect(repo.findReference('refs/heads/main')?.target()).toEqual(tip); + }); + it('extract signature from unsigned commit', async () => { const p = await useFixture('commits'); const repo = await openRepository(p);