Skip to content

fix: init setup + commit signing e2e test#46

Merged
stormer78 merged 5 commits intoOpenVTC:release/v0.1.6from
affrncsp:fix/did-git-signing-e2e
Apr 18, 2026
Merged

fix: init setup + commit signing e2e test#46
stormer78 merged 5 commits intoOpenVTC:release/v0.1.6from
affrncsp:fix/did-git-signing-e2e

Conversation

@affrncsp
Copy link
Copy Markdown

@affrncsp affrncsp commented Apr 16, 2026

Changes

Fixes seven failures across the signing and verification paths, with unit tests and follow-on improvements.


Fix 1: Signing buffer path treated as unrecognised subcommand

Symptom

error: unrecognized subcommand '/var/folders/.../T/.git_signing_buffer_tmpHl1G3w'
fatal: failed to write commit object

Root cause

Git appends the buffer file path as a trailing positional argument when it calls the signing program. The CLI had no positional field defined, so clap treated the path as an unknown subcommand.

Fix

Added a hidden positional sign_file field to Cli and threaded it through to handle_sign. To prevent a side effect of this change where any unrecognised bare word (e.g. a typo) would be silently absorbed into sign_file and fall through to the help branch with a guard was added: if sign_file is set but -Y is absent, the tool errors with an unrecognised subcommand message. The git signing path always includes -Y sign and is never affected by the guard.

Files changed: src/main.rs, src/sign.rs


Fix 2: Global user.signingKey overrides the config path passed via -f

Symptom

error: failed to parse config: /Users/you/.ssh/id_ed25519.pub

Root cause

When user.signingKey is set in the global git config, git passes that value as the -f argument instead of gpg.ssh.defaultKeyFile. did-git-sign received the .pub path and failed to parse it as its JSON config file.

Fix

did-git-sign init now sets user.signingKey at local repo scope to the did-git-sign config path, overriding any global value. Because user.signingKey is conventionally a .pub path, setting it to a JSON file may surprise users inspecting git config --list. If a global user.signingKey existed before init, a notice is now printed explaining the local override and confirming the global configuration is unchanged.

Files changed: src/init.rs, src/main.rs


Fix 3: Signature written to stdout; git expects a .sig file on disk

Symptom

-----BEGIN SSH SIGNATURE-----
...
-----END SSH SIGNATURE-----
error: failed reading ssh signing data buffer from '...tmpLam6hf.sig': No such file or directory
fatal: failed to write commit object

Root cause

The signature was printed to stdout. Git reads it back from <buffer_file>.sig on disk after the signing program exits. The .sig file was never created.

Fix

handle_sign now writes the signature to <sign_file>.sig, matching ssh-keygen behaviour.

Files changed: src/sign.rs


Fix 4: Commits show as "Unverified" on GitLab / GitHub

Symptom

Signing succeeds locally but the commit badge on GitLab or GitHub shows Unverified.

Root cause

The signing public key was not registered against the committer account, or the committer email did not match the account email.

Fix

did-git-sign init now prints explicit key registration steps after setup completes. did-git-sign health prints the SSH public key and the allowed-signers entry at any time.

Files changed: src/main.rs


Fix 5: git config user.email shows the DID key ID instead of the user's email

Symptom

$ git config user.email
did:webvh:abc:example.com#key-0

Or two user.email entries appear in git config --list, with the DID key ID as the local override.

Root cause

setup_git unconditionally wrote git config user.email <did_key_id>, clobbering the user's configured email at local repo scope.

Fix

Removed the unconditional write. did-git-sign init configures signing only and managing git identity fields is outside its scope.

Git's SSH verification is a two-step process: it first calls find-principals, which scans the allowed_signers file and returns the principal whose public key fingerprint matches the signature, which is the DID key ID. It then calls verify with that fingerprint-derived principal as -I. user.email is not part of either step. The two things that depend on email and on the allowed_signers principal are on separate paths:

Path Key input user.email used?
Local git log --show-signature Key fingerprint → find-principals → DID No
Remote forge "Verified" badge Committer email → account's registered keys Yes

This was confirmed against a live repo using git log --show-signature returned a good signature with user.email set to a real address and allowed_signers containing only the DID principal. Given this, the --email flag that was also part of this fix was removed: since user.email has no role in local verification, the flag serves no purpose. Git identity remains the user's own configuration. Any user who previously passed --email can set their email directly with git config --local user.email <address>.

❯ git log --show-signature
commit 0b9cd65e35bba0d31a203cd5789d9dea99a02612 (HEAD -> fix/did-git-signing-e2e, origin/fix/did-git-signing-e2e)
Good "git" signature for did:webvh:...#key-0 with ED25519 key SHA256:...oZ/eIQ/iTXdw
Author: <Your Name> <yourname@example.com>
Date:   Sat Apr 18 15:27:01 2026 +0800

    fix: improvements based on review + tests

    Signed-off-by: <Your Name> <yourname@example.com>

Files changed: src/init.rs, src/main.rs


Fix 6 & 7: git log --show-signature fails with "unexpected argument '-s'" and "'-O'"

Symptom

error: unexpected argument '-s' found
error: unexpected argument '-O' found

Both errors appear twice per git log --show-signature invocation because git calls the signing program separately for each verification step.

Root cause

When verifying a commit, git calls the signing program with -s <sig_file>, -I <principal>, and -O hashalg=sha512. None of these flags were declared in Cli, so clap rejected them. The "verify" operation also fell through to bail! since only "sign" was handled.

Fix

Added -s, -I, and -O as hidden CLI arguments. All -Y operations other than sign are delegated to the system ssh-keygen binary with all received flags forwarded verbatim. Verification is stateless and it only needs the public key from allowed_signers, which ssh-keygen handles natively.

Three improvements were made to the delegation path at the same time. The ssh-keygen binary is now resolved via a DID_GIT_SIGN_SSH_KEYGEN environment variable before falling back to the bare name which matters in GUI git clients and minimal CI containers where git's inherited $PATH may not include the OpenSSH tools. The delegate_to_ssh_keygen helper was refactored to return the exit code rather than calling process::exit internally, making the tokio runtime safety boundary visible at the call site. The explicit enumeration of operation names was replaced with a sign / catch-all split so that any new -Y operation git introduces in the future is forwarded automatically rather than breaking did-git-sign.

Files changed: src/main.rs


Tests

Three unit tests are added alongside the fixes. All 37 tests in the crate pass.

sign::tests::sig_path_appends_dot_sig_not_replaces_extension: Regression guard for Fix 3. Encodes the contract that .sig is appended to the full filename rather than replacing any existing extension. Protects against a future refactor using Path::with_extension("sig"), which would silently break buffer.diffbuffer.diff.sig into buffer.sig.

tests::delegate_forwards_all_flags_to_ssh_keygen: Regression guard for Fixes 6 & 7. Parses a real Cli via Cli::try_parse_from, points DID_GIT_SIGN_SSH_KEYGEN at a small mock shell script (tests/fixtures/mock_ssh_keygen.sh) that captures its argv to a temp file, and asserts every forwarded flag is present. Validates both argument assembly and the env-var override path from Fix 7.

init::tests::setup_git_never_writes_user_email: Regression guard for Fix 5. Runs setup_git against a real git init temp repository and asserts that git config --local user.email exits non-zero afterwards. Prevents user.email from being silently re-introduced in a future change.

The EnvVarGuard and CwdGuard helpers used by these tests are RAII structs: they restore the original env var or working directory on drop, keeping cleanup panic-safe. Both tests that mutate process-global state (set_var, set_current_dir) are annotated #[serial_test::serial] to prevent races with concurrently running tests. serial_test is added as a dev-only dependency and is not linked into the release binary.


PS: The commits are signed by DID-GIT-SIGN 🙂

@affrncsp affrncsp requested a review from a team as a code owner April 16, 2026 04:14
Signed-off-by: Francis Pineda <francis.p@affinidi.com>
@affrncsp affrncsp force-pushed the fix/did-git-signing-e2e branch from 94b3bef to aee69be Compare April 16, 2026 04:27
Signed-off-by: Francis Pineda <francis.p@affinidi.com>
@affrncsp affrncsp force-pushed the fix/did-git-signing-e2e branch from 49c3d82 to 9c947e4 Compare April 16, 2026 04:35
Signed-off-by: Francis Pineda <francis.p@affinidi.com>
@affrncsp affrncsp force-pushed the fix/did-git-signing-e2e branch from a111427 to f7b4582 Compare April 16, 2026 14:54
Signed-off-by: Francis Pineda <francis.p@affinidi.com>
Copy link
Copy Markdown
Contributor

@stormer78 stormer78 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@stormer78 stormer78 merged commit da0b944 into OpenVTC:release/v0.1.6 Apr 18, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants