feat(did-git-sign): add git trailer for git-host email compatibility - #25
Conversation
046ce23 to
2f3e75d
Compare
🔄 Security Rescan Update
📊 5 confirmed issues currently open on this change. 📌 See the pinned Security Review comment for full details and reports. 🔄 Automated Security Rescan Update |
80b4be4 to
0239a0f
Compare
…atibility Signed-off-by: Robert Kwolek <robert.k@affinidi.com> Signed-by-DID: did:webvh:QmNYECKwYUJExB19ucYGwRjvRGyPPk5ShGLUkxjiLLyyVm:affinidi.github.io:did-docs:robert#key-0
Signed-off-by: Robert Kwolek <robert.k@affinidi.com> Signed-by-DID: did:webvh:QmNYECKwYUJExB19ucYGwRjvRGyPPk5ShGLUkxjiLLyyVm:affinidi.github.io:did-docs:robert#key-0
Signed-off-by: Robert Kwolek <robert.k@affinidi.com> Signed-by-DID: did:webvh:QmNYECKwYUJExB19ucYGwRjvRGyPPk5ShGLUkxjiLLyyVm:affinidi.github.io:did-docs:robert#key-0
Signed-off-by: Robert Kwolek <robert.k@affinidi.com> Signed-by-DID: did:webvh:QmNYECKwYUJExB19ucYGwRjvRGyPPk5ShGLUkxjiLLyyVm:affinidi.github.io:did-docs:robert#key-0
0239a0f to
56e6b04
Compare
🛡️ AI Agentic Security Code Review1 AI-confirmed issue. Mandatory to check: 🔒 Security Code Review Report Details🛡️ Security Code Review Report — PR #25
🗺️ Scan CoverageModules scanned: 4 · with findings: 1 · files: 8 · findings: 6
Executive Summary
🔒 Security IssuesConfirmed Vulnerabilities (1)🟡 TOCTOU race between core.hooksPath check and write in install_hook_dispatcher
🧠 AI Triage:
📝 Description: install_hook_dispatcher reads the current core.hooksPath, checks it isn't owned by another tool, then later writes hook files and finally sets core.hooksPath. Between the read and the final write there is a window where the config could change (e.g. concurrent init/uninstall run, or another tool setting hooksPath) leading to inconsistent state. 🌱 Root Cause: Check-then-act pattern on external mutable state (git config) without atomicity or locking between the ownership check and the final config write. 🔎 Evidence: 🎯 Attack Scenario: A concurrent process (e.g., another init invocation or an attacker-controlled script racing with the install) changes core.hooksPath between the check and the write, causing did-git-sign to silently overwrite another tool's hook configuration or install into a directory whose ownership assumption is now false.
Generated by Agentic Sec — AI Security Validation Agent Details🛡️ Threat Model & Affect Analysis — PR #25
📋 Affect AnalysisChange SummaryMigrates did-git-sign's DID-to-commit identity binding mechanism from git's Diff: +348 / -45 lines
|
| Component | Impact | Change | What Changed |
|---|---|---|---|
| DID Identity Claim Mechanism | critical | modified | The DID identity claim moves from user.email (part of the signed git committer header) to a Signed-by-DID: trailer inside the commit mes |
| Git Hook Dispatcher / core.hooksPath Management | high | new | A new subsystem generates and installs 24 shell scripts (1 commit-msg trailer-writer + 23 delegating stubs) into a dedicated hooks directory |
| Uninstall / Cleanup Logic | medium | modified | uninstall() now also unsets did-git-sign.key and conditionally unsets core.hooksPath if it matches a freshly recomputed expected path. |
📁 File Classifications
crates/did-git-sign/src/init.rs
- Type: security
crates/did-git-sign/README.md
- Type: docs
🛡️ STRIDE Threat Model
Identified Threats (10)
🟠 STRIDE-1: Hook Directory Takeover via core.hooksPath Race in install_hook_dispatcher
| Field | Detail |
|---|---|
| Category | Tampering, Elevation of Privilege |
| Severity | High |
| Likelihood | Possible |
| CVSS | 7.3 CVSS:4.0/AV:L/AC:L/AT:P/PR:N/UI:N/VC:H/VI:H/VA:N/SC:N/SI:N/SA:N |
| Residual Severity | Medium |
| CWE | CWE-367,CWE-706 |
| CAPEC | CAPEC-27,CAPEC-25 |
| OWASP | A08:2021 - Software and Data Integrity Failures |
Description: install_hook_dispatcher in crates/did-git-sign/src/init.rs allows a local attacker to win a TOCTOU race between directory population and core.hooksPath assignment due to sequential non-atomic writes, resulting in execution of attacker-controlled hook scripts on every git operation.
Evidence: crates/did-git-sign/src/init.rs:~490-560
std::fs::create_dir_all(&hooks_dir)?;
for hook_name in STANDARD_GIT_HOOKS {
let hook_path = hooks_dir.join(hook_name);
...
write_executable_hook(&hook_path, &content)?;
}
git_config(scope, "core.hooksPath", &hooks_dir_str)?;
Attack Scenario:
- Attacker with local filesystem access (e.g. shared CI runner, multi-tenant dev box) monitors for did-git-sign init execution.
- install_hook_dispatcher() calls std::fs::create_dir_all(&hooks_dir) then loops writing each of STANDARD_GIT_HOOKS via write_executable_hook before finally calling git_config(scope, "core.hooksPath", &hooks_dir_str).
- Between directory creation and the final git_config call, attacker with write access to the hooks_dir path (e.g. predictable ~/.config/did-git-sign/hooks or .git/did-git-sign-hooks) races to replace a hook file such as pre-push or post-checkout with a malicious payload before write_executable_hook completes or before core.hooksPath is committed.
- Because write_executable_hook only refuses to clobber files that don't contain the 'Installed by did-git-sign' marker string, an attacker who pre-creates a file containing that marker string can plant persistent malicious content that survives the ownership check.
- Once core.hooksPath is set, every git operation (commit, push, merge) in the repository/machine triggers the delegating stub, executing attacker code with the victim's privileges.
- Attacker exfiltrates SSH signing key material or injects malicious commits signed by the victim's legitimate DID key.
Preconditions: Attacker has local write access to the target hooks_dir path prior to or during did-git-sign init execution., Victim runs did-git-sign init on a shared or compromised filesystem.
Existing Controls: write_executable_hook refuses to overwrite a hook file it did not create, checked via a marker string. • install_hook_dispatcher refuses to overwrite an existing core.hooksPath pointing elsewhere.
Recommended Mitigations: Populate hooks_dir contents atomically (e.g. write to temp dir then atomic rename) before setting core.hooksPath. • Use a marker beyond a matched substring, e.g. cryptographic signature or fixed file permissions, to authenticate hook ownership. • Set restrictive directory permissions (0700) on hooks_dir immediately upon creation before writing any files. • Verify hooks_dir is not a symlink and is owned by the current user before writing.
🟡 STRIDE-2: DID Trailer Injection via Whitespace/Newline Bypass in commit-msg Hook
| Field | Detail |
|---|---|
| Category | Spoofing, Tampering |
| Severity | Medium |
| Likelihood | Possible |
| CVSS | 5.9 CVSS:4.0/AV:L/AC:L/AT:P/PR:N/UI:N/VC:N/VI:H/VA:N/SC:N/SI:N/SA:N |
| Residual Severity | Low |
| CWE | CWE-20,CWE-93 |
| CAPEC | CAPEC-267,CAPEC-153 |
| OWASP | A03:2021 - Injection |
Description: The generated commit-msg hook script in COMMIT_MSG_HOOK allows a local attacker to spoof or corrupt the Signed-by-DID trailer due to insufficient DID value sanitization beyond whitespace stripping, resulting in a forged or malformed identity claim being embedded into signed commit content.
Evidence: crates/did-git-sign/src/init.rs:~560-600
DID=$(printf '%s' "${DID_GIT_SIGN_KEY:-}" | tr -d '\r\n')
[ -z "$DID" ] && DID=$(git config did-git-sign.key 2>/dev/null)
[ -z "$DID" ] && exit 0
case "$DID" in
did:*) ;;
*) exit 0 ;;
esac
Attack Scenario:
- Attacker sets DID_GIT_SIGN_KEY or did-git-sign.key to a value that begins with 'did:' but contains embedded control characters, git trailer syntax delimiters, or additional newline-adjacent trailer-like content that the case statement's whitespace check ([[:space:]]) does not fully cover (e.g. carriage-return-stripped but embedded colon-based trailer injection like 'did:x:evil\nSigned-off-by: attacker a@a').
- The DID value passes the 'did:*' prefix check and the whitespace rejection since tr -d only strips literal CR/LF from the DID_GIT_SIGN_KEY env var but the git config value path (git config did-git-sign.key) is not passed through the same tr -d sanitization in the shown hook logic.
- git interpret-trailers --trailer "Signed-by-DID: $DID" "$msg_file" is invoked with the attacker-controlled DID string interpolated unquoted-adjacent into the trailer value, potentially allowing trailer-block confusion if the DID string itself resembles additional trailer key:value pairs.
- verify-trust later parses the Signed-by-DID trailer and may misidentify the intended DID or accept a crafted value that resolves to an attacker-controlled DID document, given lib.rs parsing logic was not available for full review.
- A commit gets a Signed-by-DID trailer claiming an identity that does not match operator intent, and since the trailer sits inside the signed payload, the forged claim survives signature verification with the legitimate key, causing CI to attribute a commit to a different persona than the actual key owner intended.
Preconditions: Attacker controls the local git config did-git-sign.key or DID_GIT_SIGN_KEY environment variable, e.g. via a compromised shell profile or CI environment variable injection., verify-trust's trailer parser does not independently re-validate the DID format beyond what the hook checks.
Existing Controls: Hook enforces 'did:*' prefix. • Hook rejects DIDs containing whitespace via a case pattern match. • git interpret-trailers with --if-exists doNothing avoids clobbering an existing trailer.
Recommended Mitigations: Sanitize the git-config-sourced DID value with the same tr -d '\r\n' pass applied to the env var. • Validate DID syntax against a strict regex (e.g. RFC 3986-compliant DID method syntax) rather than only a prefix and whitespace check. • Have verify-trust strictly validate trailer key/value structure and reject multi-line or control-character-laden trailer values. • Quote and escape the DID value defensively before passing to git interpret-trailers.
🟠 STRIDE-3: Silent Downgrade to Unsigned Identity Claim via --no-verify Bypass
| Field | Detail |
|---|---|
| Category | Spoofing, Repudiation, Tampering |
| Severity | High |
| Likelihood | Likely |
| CVSS | 7.1 CVSS:4.0/AV:L/AC:L/AT:N/PR:N/UI:N/VC:N/VI:H/VA:N/SC:N/SI:N/SA:N |
| Residual Severity | Medium |
| CWE | CWE-354,CWE-345 |
| CAPEC | CAPEC-668,CAPEC-580 |
| OWASP | A08:2021 - Software and Data Integrity Failures |
Description: git commit --no-verify in a repository configured with did-git-sign allows a legitimate or malicious local user to bypass the commit-msg hook entirely due to git's own hook-bypass mechanism not being blocked by did-git-sign, resulting in a signed commit whose user.email diverges from the actual signing key without the compensating Signed-by-DID trailer.
Evidence: crates/did-git-sign/README.md:~205-215
Signing still refuses a commit whose claim and key disagree, naming both
halves, rather than writing one that fails in CI as `unknownKey`. That now only
happens if you write a `Signed-by-DID:` trailer yourself, or commit with the
hook bypassed (`--no-verify`) in a repo whose `user.email` is a differ
Attack Scenario:
- Repository is configured via did-git-sign init with did-git-sign.key set and core.hooksPath pointing at the dispatcher, per the diff in init.rs.
- Attacker (or compromised CI script) runs
git commit --no-verify -m "…", which git itself skips execution of the commit-msg hook (a native git feature, not specific to did-git-sign) as documented in the README's own admission: 'That now only happens if you write a Signed-by-DID: trailer yourself, or commit with the hook bypassed (--no-verify) in a repo whose user.email is a different DID.' - commit.gpgsign=true still causes
sign(COMP-001, EP-003) to be invoked and produce a valid sshsig signature over the commit, since sign.rs's own consistency check (claim vs. key) was not shown but the README states signing 'refuses a commit whose claim and key disagree' — however this refusal logic operates on user.email vs. selected key, and with the redesign, user.email is deliberately left as an ordinary address, decoupling it from the DID entirely. - The commit is pushed with a valid signature but no Signed-by-DID trailer, and CI's verify-trust step (EP-006) is expected to reject it as noSignerDid — but if any CI path caches prior verification state, uses a permissive fallback for legacy user.email-based DIDs, or the repository allows re-tagging/force-push after a manual bypass of CI gating, the commit could be merged.
- Result: a valid Ed25519 signature exists on a commit with no verifiable DID claim, undermining the entire non-repudiation guarantee the system is built to provide, and creating a repudiation vector where the actual committer can later deny authorship since the trailer-based provenance chain has a gap.
Preconditions: Local user has commit access and chooses (or is scripted) to pass --no-verify., CI enforcement of the noSignerDid check is the only backstop; no server-side pre-receive hook independently verifies the trailer.
Existing Controls: verify-trust is documented to reject commits lacking a DID claim as noSignerDid. • README explicitly documents this bypass path as a known limitation.
Recommended Mitigations: Enforce Signed-by-DID trailer validation via a server-side pre-receive hook in addition to client-side commit-msg hook, since client-side hooks are inherently bypassable. • Document and default to branch protection rules requiring the noSignerDid CI check to pass before merge, with no override capability for standard contributors. • Add a corroborating CI check that cross-references commit.gpgsign metadata against the presence of the trailer, flagging any signed-but-untrailered commit as high-priority for manual review.
🟡 STRIDE-4: Legacy user.email Fallback Enables DID Downgrade Attack in verify-trust
| Field | Detail |
|---|---|
| Category | Spoofing, Tampering |
| Severity | Medium |
| Likelihood | Possible |
| CVSS | 6.4 CVSS:4.0/AV:N/AC:L/AT:N/PR:N/UI:N/VC:N/VI:H/VA:N/SC:N/SI:N/SA:N |
| Residual Severity | Low |
| CWE | CWE-706,CWE-346 |
| CAPEC | CAPEC-141,CAPEC-151 |
| OWASP | A07:2021 - Identification and Authentication Failures |
Description: verify-trust's fallback path for 'older commits that carry the DID in user.email' allows an attacker to force acceptance of a downgraded identity-claim mechanism due to dual code paths for DID discovery, resulting in inconsistent trust semantics that could be exploited to mask a missing or forged trailer with a spoofed user.email.
Evidence: crates/did-git-sign/README.md:~30-33
A `commit-msg` hook installed by `init` writes
it; older commits that carry the DID in `user.email` still verify, through a
fallback in `verify-trust`.
Attack Scenario:
- verify-trust (COMP-002, EP-006) — per README — 'older commits that carry the DID in user.email still verify, through a fallback in verify-trust', meaning the verifier must accept two structurally different sources of identity claim: the Signed-by-DID trailer (new) and user.email (legacy).
- An attacker crafts a new commit (not an old legacy one) whose committer user.email is deliberately set to a DID string that they do not fully control the corresponding key for, or that partially collides with an intended target DID, and does not include a Signed-by-DID trailer.
- If verify-trust's fallback logic determines DID-ness by pattern-matching user.email (e.g. checking for a 'did:' prefix) without distinguishing 'commit predates the trailer feature' from 'commit simply omitted the trailer', the fallback path is reachable by any commit, not just legacy ones — since git commit timestamps and commit-msg hook installation dates are not cryptographically bound to feature availability.
- This allows an attacker to intentionally choose the legacy path (weaker validation surface, potentially different key-matching logic, or different resolution caching) over the modern trailer path, exploiting whichever fallback branch has weaker or differently-scoped checks (e.g. no consistency check that the committer's key matches the DID stated only in user.email, if
signwas bypassed). - Because the full verify-trust source (crates/verify-trust/src/lib.rs) was not available in the reduced context, the exact resolution-and-caching behavior for each path cannot be confirmed, but the dual-path design is inherently a larger attack surface than a single canonical identity-claim mechanism.
Preconditions: verify-trust implements a legacy-compatible fallback that trusts user.email as a DID claim without provably restricting it to genuinely pre-trailer-era commits., Attacker can create git commits with arbitrary committer email content.
Existing Controls: Fallback is explicitly documented as intended only for pre-existing legacy commits. • Modern flow decouples user.email from DID entirely, reducing new commits' reliance on the fallback.
Recommended Mitigations: Restrict the legacy user.email fallback to commits authored before a specific cutover commit/tag/date, verified cryptographically (e.g. via a repository-wide feature-flag commit marker) rather than accepting it universally. • Require the same key-to-DID binding validation for the legacy path as for the trailer path, so no downgrade in verification rigor exists between the two. • Emit a deprecation warning or CI annotation whenever the legacy fallback path is used on a commit newer than the trailer rollout, to enable monitoring for abuse.
🟡 STRIDE-5: Symlink/Path Traversal in expected_hooks_dir via git rev-parse Output
| Field | Detail |
|---|---|
| Category | Tampering, Elevation of Privilege |
| Severity | Medium |
| Likelihood | Unlikely |
| CVSS | 5.3 CVSS:4.0/AV:L/AC:L/AT:P/PR:N/UI:N/VC:L/VI:L/VA:N/SC:N/SI:N/SA:N |
| Residual Severity | Low |
| CWE | CWE-59,CWE-706 |
| CAPEC | CAPEC-132,CAPEC-17 |
| OWASP | A01:2021 - Broken Access Control |
Description: expected_hooks_dir in crates/did-git-sign/src/init.rs allows a local attacker controlling the working directory or a maliciously crafted .git structure to redirect the hook installation path due to unsanitized trust in git rev-parse --absolute-git-dir output, resulting in writing hook files to an attacker-influenced location outside the intended repository.
Evidence: crates/did-git-sign/src/init.rs:~330-345
let output = Command::new("git")
.args(["rev-parse", "--absolute-git-dir"])
.output()
.context("failed to find .git directory")?;
let git_dir = PathBuf::from(String::from_utf8_lossy(&output.stdout).trim());
Ok(Some(git_dir.join("did-git-sign-hooks")))
Attack Scenario:
- Attacker sets up a malicious repository or git worktree structure (e.g. a .git file pointing to a symlinked or crafted gitdir, as permitted by git worktrees/submodules) such that
git rev-parse --absolute-git-dirreturns an attacker-influenced path. - Victim runs did-git-sign init inside this crafted repository, and expected_hooks_dir(false) trusts the command output verbatim:
let git_dir = PathBuf::from(String::from_utf8_lossy(&output.stdout).trim()); Ok(Some(git_dir.join("did-git-sign-hooks"))). - install_hook_dispatcher then calls std::fs::create_dir_all(&hooks_dir) and writes hook files into this attacker-influenced path, which — if it resolves via a symlink to a location such as a shared system directory, another user's home directory, or an unrelated repository the victim also uses — plants attacker-controlled or victim-authored hook scripts in an unintended location.
- Because core.hooksPath is then set to reference this path, subsequent git operations in the crafted repository (or any repository the symlink target ultimately resolves to) execute the dispatcher, potentially exposing signing key usage patterns or corrupting hooks for other tooling.
- This is a lower-severity variant since it requires the victim to already be operating inside an attacker-supplied repository structure, but is a realistic supply-chain scenario (e.g. cloning a malicious repo and running init inside it).
Preconditions: Victim runs did-git-sign init inside an attacker-crafted or attacker-influenced git repository/worktree structure., git rev-parse --absolute-git-dir can be influenced by .git file redirection, worktrees, or GIT_DIR environment variable manipulation.
Existing Controls: Command output is trimmed of whitespace before use, avoiding trivial injection via trailing content. • create_dir_all requires filesystem-level write permission, limiting blast radius to accessible paths.
Recommended Mitigations: Canonicalize and validate that the resolved git_dir is within an expected repository boundary before joining and writing to it. • Reject GIT_DIR/rev-parse output that resolves through symlinks to paths outside the current working tree without explicit confirmation. • Run hook installation with the least-privilege filesystem context possible.
🟡 STRIDE-6: core.hooksPath Ownership Check Race Enables Silent Hijack of Existing Hook Managers
| Field | Detail |
|---|---|
| Category | Tampering, Denial of Service |
| Severity | Medium |
| Likelihood | Possible |
| CVSS | 6.1 CVSS:4.0/AV:L/AC:L/AT:P/PR:N/UI:N/VC:N/VI:H/VA:N/SC:N/SI:N/SA:N |
| Residual Severity | Low |
| CWE | CWE-362,CWE-367 |
| CAPEC | CAPEC-25 |
| OWASP | A04:2021 - Insecure Design |
Description: install_hook_dispatcher in crates/did-git-sign/src/init.rs allows a race condition between reading and later relying on core.hooksPath ownership due to a check-then-act pattern without locking, resulting in a window where a concurrently-running process (e.g. husky/lefthook installer) could take ownership of the same config key, leading to hook execution inconsistency or takeover.
Evidence: crates/did-git-sign/src/init.rs:~505-520
if let Some(existing) = git_config_get(scope, "core.hooksPath")?
&& Path::new(existing.trim()) != hooks_dir
{
anyhow::bail!(
"{scope} core.hooksPath is already set to '{existing}'; refusing to overwrite it. ..."
);
}
Attack Scenario:
- install_hook_dispatcher reads the current core.hooksPath via git_config_get(scope, "core.hooksPath") and, finding it either unset or equal to its own expected path, proceeds to populate hooks_dir and later call git_config(scope, "core.hooksPath", &hooks_dir_str).
- Concurrently, another process (e.g. a package manager postinstall script running husky install, or a second did-git-sign init invocation triggered by a build script) also checks and sets core.hooksPath during this same window.
- Because there is no file lock or atomic compare-and-swap on the git config value between the check and the final set, both processes can believe they successfully claimed the slot, and the last writer wins non-deterministically.
- If husky's hooks path wins the race after did-git-sign has already written its dispatcher scripts (or vice versa), the resulting core.hooksPath may point to a directory that does not contain the Signed-by-DID trailer logic, silently disabling commit provenance enforcement without any error being surfaced to the user.
- This is particularly impactful in CI/CD pipelines or automated dev-environment bootstrap scripts where multiple setup tools run in parallel or in a non-deterministic order across cache-warm builds.
Preconditions: Multiple hook-managing tools (did-git-sign, husky, lefthook, pre-commit) run install/init concurrently or in a race-prone bootstrap sequence., No file-system or git-level lock coordinates core.hooksPath writes across tools.
Existing Controls: install_hook_dispatcher refuses to proceed if core.hooksPath is already set to a different value at the time of the check. • The refusal path emits a clear, actionable error message naming the conflict.
Recommended Mitigations: Use git config --local --replace-all with an optimistic lock pattern (read core.hooksPath immediately before write, abort if changed) or a filesystem lock file during install. • Document that concurrent hook-manager installation is unsupported and recommend serializing setup steps in CI bootstrap scripts. • Add a post-install verification step that re-reads core.hooksPath after installation completes and warns if it no longer matches the expected dispatcher path.
🟡 STRIDE-7: Non-Fatal Hook Install Failure Leaves Warning-Only Signal for Signing Continuity
| Field | Detail |
|---|---|
| Category | Tampering, Repudiation, Denial of Service |
| Severity | Medium |
| Likelihood | Likely |
| CVSS | 5.5 CVSS:4.0/AV:L/AC:L/AT:N/PR:N/UI:N/VC:N/VI:L/VA:L/SC:N/SI:N/SA:N |
| Residual Severity | Low |
| CWE | CWE-755,CWE-392 |
| CAPEC | CAPEC-548 |
| OWASP | A09:2021 - Security Logging and Monitoring Failures |
Description: The install() function in crates/did-git-sign/src/init.rs allows a hook-installation failure to be silently downgraded to a stderr warning due to non-fatal error handling in the install_hook_dispatcher call site, resulting in operators mistakenly believing DID-trailer provenance is active when it is not enforced client-side.
Evidence: crates/did-git-sign/src/init.rs:~107-124
if let Err(e) = install_hook_dispatcher(args.global) {
eprintln!(
"warning: could not install the git hook that writes the Signed-by-DID trailer:\n {e}\n ..."
);
}
Attack Scenario:
- install_hook_dispatcher() fails (e.g. because core.hooksPath is already claimed by husky, or filesystem permissions deny directory creation), and the call site in install() catches this via
if let Err(e) = install_hook_dispatcher(args.global) { eprintln!(...) }rather than propagating the error and aborting the overall init. - The rest of
did-git-sign initcompletes successfully: SSH signing keys are generated, commit.gpgsign is enabled, allowed_signers is configured, and did-git-sign.key is set — everything needed forsignto produce valid signatures. - An operator or CI bootstrap script that only checks the exit code of
did-git-sign init(a common automation pattern) sees success (exit 0) and proceeds, unaware that the commit-msg hook — the only mechanism that writes the Signed-by-DID trailer — was never installed. - Every subsequent commit is validly signed (Ed25519/sshsig) but carries no DID trailer, and depending on the CI verify-trust check's strictness and whether the legacy user.email fallback is coincidentally satisfied, commits may either fail loudly (noSignerDid) after several commits have accumulated, or — worse — silently pass if user.email happens to still be a stale DID from a prior config, per the interaction with STRIDE-4.
- Root-caused only at CI time, well after the developer began committing, creating wasted work, confusing failures, and potential accidental merges if CI gating is imperfect.
Preconditions: Automation or CI treats did-git-sign init exit code as the sole success signal without parsing stderr for the specific hook-install warning., core.hooksPath conflict or filesystem permission issue exists at install time.
Existing Controls: The warning message is detailed and actionable, naming the specific failure and remediation steps. • The warning explicitly states that sign will refuse to sign without a DID claim (per the code comment), which is a compensating control if accurate.
Recommended Mitigations: Make install() return a non-zero exit code (or a structured warning field surfaced prominently, not just via eprintln!) when a security-relevant sub-step like hook installation fails. • Provide a did-git-sign doctor or --verify subcommand that CI bootstrap scripts can run to positively confirm hook installation and core.hooksPath alignment before relying on it. • Log hook-install failures to a machine-readable status file that CI can assert on, in addition to the human-readable stderr message.
🔵 STRIDE-8: Uninstall Path Confusion Leaves Stale core.hooksPath Referencing Removed Directory
| Field | Detail |
|---|---|
| Category | Tampering, Denial of Service |
| Severity | Low |
| Likelihood | Possible |
| CVSS | 4.5 CVSS:4.0/AV:L/AC:L/AT:P/PR:N/UI:N/VC:N/VI:L/VA:L/SC:N/SI:N/SA:N |
| Residual Severity | Low |
| CWE | CWE-459,CWE-704 |
| CAPEC | CAPEC-462 |
| OWASP | A04:2021 - Insecure Design |
Description: unset_did_git_sign_hooks_path in crates/did-git-sign/src/init.rs allows core.hooksPath to be left dangling due to reliance on an exact path-equality check against a freshly recomputed expected_hooks_dir value, resulting in git silently falling back to default hook behavior (or erroring on missing hooksPath) after uninstall if the recomputed path differs from what was actually configured (e.g. after a HOME change, global/local mismatch, or manual edits).
Evidence: crates/did-git-sign/src/init.rs:~307-317
fn unset_did_git_sign_hooks_path(scope: &str, global: bool) -> Result<bool> {
let Some(expected) = expected_hooks_dir(global)? else { return Ok(false); };
let Some(configured) = git_config_get(scope, "core.hooksPath")? else { return Ok(false); };
if Path::new(configured.trim()) == expect
Attack Scenario:
- Operator runs
did-git-sign uninstall, which calls unset_did_git_sign_hooks_path(scope, global) to decide whether to remove core.hooksPath. - The function recomputes
expected_hooks_dir(global)fresh at uninstall time and compares it via exact PathBuf equality against the currently configured core.hooksPath value. - If the environment has changed since install (e.g. XDG_CONFIG_HOME or HOME changed, the repository was moved, or the user previously ran init with --global and later locally, creating scope ambiguity), the recomputed expected path no longer matches the configured value even though the configured value was in fact set by did-git-sign.
- unset_did_git_sign_hooks_path returns Ok(false) (not equal, do nothing), silently leaving core.hooksPath pointing at a directory that install's own hooks_dir removal (if any file cleanup logic exists elsewhere) may have deleted or that the uninstall flow otherwise no longer maintains.
- Subsequent git operations either execute stale/removed hook scripts (if the directory still physically exists but is orphaned) or fail/no-op if git can't find the configured hooksPath, silently disabling all hook-based enforcement (Signed-by-DID trailer writing) without any explicit user notification, since this is a distinct code path from the install-time warning in STRIDE-7.
Preconditions: User's environment (HOME, XDG paths) or scope (global vs local) differs between install time and uninstall time., No file cleanup step independently reconciles core.hooksPath against actual filesystem hook directories.
Existing Controls: The comparison logic exists at all (some implementations skip verification entirely and blindly unset). • Errors reading core.hooksPath are surfaced as warnings rather than silently swallowed.
Recommended Mitigations: Store the originally-configured hooks_dir path in a metadata file at install time and read it back at uninstall time instead of recomputing it, to avoid drift. • Warn explicitly when core.hooksPath is set but does not match the expected path, rather than silently no-op'ing. • Provide a did-git-sign status command showing current hook configuration state versus expected state for troubleshooting.
🟠 STRIDE-9: Global core.hooksPath Cross-Repository Contamination Executes Untrusted Hooks Machine-Wide
| Field | Detail |
|---|---|
| Category | Tampering, Elevation of Privilege, Information Disclosure |
| Severity | High |
| Likelihood | Likely |
| CVSS | 7.6 CVSS:4.0/AV:L/AC:L/AT:N/PR:N/UI:N/VC:N/VI:H/VA:N/SC:N/SI:N/SA:N |
| Residual Severity | Medium |
| CWE | CWE-668,CWE-807 |
| CAPEC | CAPEC-176,CAPEC-586 |
| OWASP | A04:2021 - Insecure Design |
Description: the --global install path in crates/did-git-sign/src/init.rs allows every repository on the machine to be silently subjected to the did-git-sign hook dispatcher due to global core.hooksPath scope, resulting in the delegating hook logic executing (and the commit-msg trailer logic firing) for repositories the user did not intend to participate in DID-based signing, including potentially untrusted cloned repositories.
Evidence: crates/did-git-sign/README.md:~78-84
This also sets `did-git-sign.key` and `core.hooksPath` for **every repository
on the machine** — that pair decides the identity your commits claim, and it
must match the key that signs them. Right for one community; wrong for two...
Attack Scenario:
- User runs
did-git-sign init --global, which per the README and code sets a global core.hooksPath pointing at ~/.config/did-git-sign/hooks, containing the dispatcher for all 24 standard hooks including pre-commit, post-checkout, pre-push, and post-merge. - User later clones an untrusted third-party repository that itself relies on core.hooksPath being unset or pointing at its own tooling (e.g. a repo bundling its own security scanning pre-commit setup expecting to configure core.hooksPath itself during its own setup script).
- Because the global setting takes precedence unless overridden locally, and did-git-sign's
init'refuses to take core.hooksPath if something else already owns it' only at the time did-git-sign itself is being installed — not retroactively when a new repository is later cloned — the untrusted repository's own hook-setup tooling either silently fails to take core.hooksPath (since did-git-sign already owns it globally) or, worse, the delegating stub in did-git-sign's dispatcher execs the repository's own .git/hooks/ per its design, meaning any malicious hook scripts committed directly into that untrusted repo's .git/hooks (which is not normally tracked by git and must be manually placed, but could be provisioned via a malicious bootstrap/install script bundled with the repo) get executed transparently through the delegation chain. - This creates an unexpected trust bridge: the delegating hooks make it appear to any repository-specific tooling as though hooks 'just work' as normal, masking the fact that a machine-wide interception layer is present, which could be leveraged by a malicious repository's setup script to detect (via probing behavior differences) that did-git-sign is active and adjust its attack accordingly, or simply rely on the delegation to ensure its own malicious .git/hooks payload still executes exactly as if core.hooksPath were unset.
- Separately, and more directly: cloning an untrusted repo and running any git command that triggers a hook (e.g. git commit, git merge) inside it will fire commit-msg and inject a Signed-by-DID trailer using the globally-configured DID key — meaning the user's DID identity is asserted (and cryptographically signed) on commits made in repositories the user may not intend to associate with that persona at all, especially for pseudonymous or context-separated identities as the README's whole 'community persona' discussion is designed to prevent.
Preconditions: User has run --global install of did-git-sign., User subsequently interacts with repositories they did not intend to bind to the globally-configured DID persona.
Existing Controls: README explicitly warns: 'Right for one community; wrong for two, and quietly so, since commits in the other community would claim this DID.' • init prints a per-remote alternative recommendation when --global is used. • Delegating hooks preserve repository-specific hook execution, reducing (but not eliminating) functional breakage.
Recommended Mitigations: Default to local (--local) installation and require an explicit, loudly-confirmed opt-in for --global, given the cross-repository identity-binding risk. • Provide a per-repository allow/deny list so the global dispatcher only injects the Signed-by-DID trailer in repositories the user has explicitly opted into, rather than every repository on the machine. • Detect and warn when a git command is run inside a repository lacking any prior did-git-sign local configuration but inheriting the global hooksPath, prompting the user to confirm persona binding on first use per repository.
🟡 STRIDE-10: Missing Input Validation on DID Value Allows Trailer Block Structural Confusion in git interpret-trailers
| Field | Detail |
|---|---|
| Category | Tampering, Information Disclosure |
| Severity | Medium |
| Likelihood | Possible |
| CVSS | 5.7 CVSS:4.0/AV:L/AC:L/AT:P/PR:N/UI:N/VC:N/VI:H/VA:N/SC:N/SI:N/SA:N |
| Residual Severity | Low |
| CWE | CWE-20,CWE-150 |
| CAPEC | CAPEC-267,CAPEC-136 |
| OWASP | A03:2021 - Injection |
Description: the COMMIT_MSG_HOOK script in crates/did-git-sign/src/init.rs allows an attacker who controls the local did-git-sign.key git config value to inject additional trailer-like lines due to relying solely on a 'did:' prefix check and a whitespace-only rejection filter, resulting in potential trailer-block structure manipulation that could confuse downstream automated parsers (verify-trust or other CI tooling) reading the commit message trailers.
Evidence: crates/did-git-sign/src/init.rs:~583-596
case "$DID" in
*[[:space:]]*)
echo "did-git-sign: the selected DID contains whitespace; refusing to write a trailer" >&2
exit 1
;;
esac
git interpret-trailers --in-place --if-exists doNothing \
--trailer "Signed-by-DID: $DID" "$msg_file" || exit 1
Attack Scenario:
- Attacker with local write access to a shared or CI-managed repository's git config (e.g. via a malicious pre-existing .git/config committed accidentally, or a build script that sets local config from an untrusted source) sets
did-git-sign.keyto a crafted string starting with 'did:' but containing embedded non-whitespace structural characters recognized by git's trailer syntax, such as colons or the RFC 822-style 'Key: Value' pattern repeated (e.g. 'did:web:evil.example\x01Reviewed-by: attacker a@a' where \x01 is a non-whitespace separator git interpret-trailers might still parse across, depending on trailer separator configuration). - The whitespace check
case "$DID" in *[[:space:]]*) ... exit 1 ;; esaconly catches POSIX [:space:] characters (space, tab, newline, CR, FF, VT) and does not reject other control characters or Unicode look-alikes that could still be interpreted specially by downstream consumers or terminal rendering (e.g. ANSI escape sequences for log-injection into CI console output). git interpret-trailers --trailer "Signed-by-DID: $DID"embeds this value verbatim into the commit message trailer block; because git's trailer parsing is line-oriented and configurable (trailer.separators), an attacker who knows or influences the repository's trailer.* configuration could craft a DID value that, when combined with specific separator configs, causes the resulting trailer block to be mis-parsed as multiple trailers instead of one.- Downstream consumers such as verify-trust (COMP-002) or third-party changelog/release-note generators that also parse git trailers could then be confused into associating an attacker-injected key/value pair (e.g. a fake 'Reviewed-by' or 'Signed-off-by') with the commit, laundering a false attestation through the legitimate signing pipeline.
- Even absent full trailer confusion, unfiltered control characters (e.g. ANSI escape codes) embedded in the DID value that flow into CI log output when tooling prints 'Signed-by-DID: ' create a terminal/log injection vector (CWE-150-adjacent), potentially misleading operators reviewing CI logs.
Preconditions: Attacker can set the local did-git-sign.key git config value (requires local write access or a supply-chain foothold in a build/config script)., verify-trust or other consumers do not perform independent strict DID syntax validation before trusting the trailer content.
Existing Controls: Prefix check requires the value start with 'did:'. • POSIX whitespace characters are rejected outright. • git interpret-trailers --if-exists doNothing prevents duplicate trailers from the hook's own re-runs.
Recommended Mitigations: Apply a strict allow-list character validation (e.g. only alphanumerics, colons, periods, hyphens per DID method-name/method-specific-id syntax) instead of a blocklist-style whitespace check. • Reject any control character (0x00-0x1F, 0x7F) in the DID value, not just POSIX space-class characters. • Have verify-trust independently re-validate the DID syntax with a strict parser/grammar before resolving it, never trusting the raw trailer value structurally.
🍝 PASTA Threat Model
Application Purpose
did-git-sign binds decentralized identifiers (DIDs) to git commits via SSH-signed sshsig signatures and a Signed-by-DID trailer, enabling verify-trust and CI pipelines to cryptographically attribute commits to decentralized identities rather than centralized forge accounts.
Inherent Risks
- Client-side git hooks are fundamentally bypassable by any user with --no-verify or direct signature-forging access to their own repository.
- A single core.hooksPath slot per repository/machine creates unavoidable contention with other hook-management tooling.
- Trust in the DID-to-commit binding is only as strong as the weakest verification path (legacy user.email fallback vs. modern trailer), and dual-path designs inherently widen the attack surface.
- Global installation scope conflates identity binding across unrelated repositories and communities.
Objectives
Risk: Treat any bypass of the DID-claim mechanism (e.g. --no-verify) as an accepted residual risk requiring compensating server-side controls.; Treat core.hooksPath contention as a configuration risk requiring explicit conflict detection rather than silent overwrite.
Business: Provide cryptographically verifiable, decentralized commit provenance as a differentiator for trust-sensitive open-source and consortium software supply chains.
Security: Ensure every signed commit carries a verifiable, non-forgeable DID claim inside the signed payload.; Prevent a mismatch between the signing key and the claimed identity from ever producing a commit that appears valid.
Financial: Avoid costly incident response and reputational remediation from a supply-chain compromise traced to unverifiable commit authorship.
Compliance: Support auditability requirements for software supply-chain integrity frameworks (e.g. SLSA, in-toto) via cryptographic commit provenance.
Functional: Sign commits with SSH keys bound to DIDs and write a tamper-evident identity trailer without breaking existing forge-based (GitHub/GitLab) account attribution.
Operational: Support multiple community personas per contributor without configuration drift between the signing key and the identity claim.; Coexist with existing hook-management tools (husky, lefthook, pre-commit) without silently disabling them.
Business Impact Analysis (2)
BIA-1: Commit Provenance and Identity Attestation (Critical)
The end-to-end process of generating a DID-bound SSH signature and Signed-by-DID trailer at commit time, then verifying that binding in CI before merge.
MTD: 01 days 00:00 hours | RTO: 00 days 04:00 hours | RPO: 00 days 00:00 hours
- Stakeholders: CI/CD Pipeline Operators / Consortium Governance Bodies / Contributors / Forge Platform (GitHub/GitLab) / Security/Compliance Auditors
- Dependencies: git CLI / gpg.ssh signing subsystem / verify-trust CI check / DID document resolution service (webvh method)
- Disruptions: core.hooksPath conflict silently disables trailer injection / --no-verify bypass produces signed but unattributed commits / Global install cross-contaminates unrelated repositories' identity claims
- Impacts: Loss of non-repudiation for merged commits, undermining supply-chain audit trail / False attribution of commits to the wrong community persona / CI false-negative allowing unverifiable commits to merge
BIA-2: Git Hook Lifecycle Management (High)
Installation, coexistence, and removal of the commit-msg trailer hook and delegating stubs for all standard git hooks via core.hooksPath.
MTD: 03 days 00:00 hours | RTO: 00 days 08:00 hours | RPO: 00 days 00:00 hours
- Stakeholders: Contributors / DevOps/Platform Engineering / Third-Party Hook Tooling Vendors (husky, lefthook, pre-commit)
- Dependencies: core.hooksPath git configuration slot / Local filesystem write access to hooks directory / git rev-parse for git-dir resolution
- Disruptions: Race condition during hook directory population / Ownership conflict with existing hook managers / Stale core.hooksPath after uninstall due to path recomputation drift
- Impacts: Other tooling's hooks silently stop running / Provenance enforcement silently disabled / Operator confusion requiring manual remediation
Technical Scope
Roles (1):
Actors (3): AC-1 Contributor · AC-2 CI Pipeline Service · AC-3 Third-Party Hook Manager (husky/lefthook/pre-commit)
Entry Points (6): EP-001 CLI init command · EP-002 CLI uninstall command · EP-003 CLI sign command (git gpg.ssh.program) · EP-004 commit-msg hook trigger · EP-005 Delegating hook stubs (all standard hooks) · EP-006 verify-trust library call
Threat Actors (3): TA-1 Malicious Insider Contributor · TA-2 Local Multi-Tenant Attacker · TA-3 Malicious Repository Publisher
Infrastructure (2): IF-1 Contributor Local Machine · IF-2 CI/CD Runner Infrastructure
Trust Boundaries (4): TB-1 Local Developer Workstation · TB-2 Local Git Repository Filesystem · TB-3 CI/CD Pipeline · TB-4 External DID Resolution Network
External Entities (3): EE-1 Contributor Workstation User · EE-2 CI/CD Runner · EE-3 DID Resolution Service (webvh)
System Components (5): SC-1 did-git-sign CLI (init/uninstall/sign) · SC-2 Generated Git Hook Dispatcher · SC-3 verify-trust CI Verifier · SC-4 Local Git Config Store · SC-5 DID Document Registry (webvh)
Resources And Assets (4): RA-1 Ed25519 SSH Signing Private Key · RA-2 did-git-sign.key / core.hooksPath Git Config · RA-3 Signed-by-DID Git Trailer · RA-4 allowed_signers File
Technologies And Dependencies (5): TD-1 anyhow · TD-2 base64 · TD-3 dirs · TD-4 git CLI · TD-5 POSIX sh
Use Cases (3)
- Persona-Bound Commit Signing: A contributor commits code; git invokes did-git-sign sign as the gpg.ssh.program, which signs the commit with the persona's Ed25519 key while the commit-msg hook writes the Signed-by-DID trailer, prod
- CI Commit Provenance Verification: A CI pipeline runner checks out a pushed commit and invokes verify-trust to parse the Signed-by-DID trailer, resolve the claimed DID against the external registry, and confirm the resolved key matches
- Hook Dispatcher Installation and Coexistence: An operator runs did-git-sign init, which checks whether core.hooksPath is already owned by another tool, then populates a hooks directory with a commit-msg trailer script and 23 delegating stubs so e
📋 Risk Registry (5)
| ID | Title | Severity | Residual | Priority | Effort |
|---|---|---|---|---|---|
| RISK-1 | Shared/multi-tenant machines allow local attackers to race or hijack hook installation and steal signing key usage | High | Medium | Short-Term | Medium |
| RISK-2 | Client-side-only enforcement of the DID claim means --no-verify or missing server-side checks allow signed-but-unattributed commits to merge | High | Medium | Immediate | High |
| RISK-3 | Global installation scope silently binds a contributor's DID identity to every repository on the machine, including untrusted third-party clones | High | Medium | Short-Term | Medium |
| RISK-4 | Insufficient DID value sanitization creates trailer/log injection surface reachable via local git config or environment variables | Medium | Low | Medium-Term | Low |
| RISK-5 | Non-fatal error handling and stale-state drift in hook lifecycle management cause silent loss of provenance enforcement | Medium | Low | Medium-Term | Medium |
⚔️ Attack Scenarios (4)
SC-1: did-git-sign CLI (init/uninstall/sign)
---
config:
layout: dagre
look: classic
theme: dark
---
flowchart LR
subgraph SL1["1. System Component"]
direction LR
SC1@{ shape: rect, label: "SC-1: did-git-sign CLI" }
end
subgraph SL2["2. Weaknesses"]
direction LR
CWE367@{ shape: rect, label: "CWE-367: TOCTOU Race Condition" }
CWE362@{ shape: rect, label: "CWE-362: Concurrent Execution Race" }
end
subgraph SL3["3. Attack Patterns"]
direction LR
CAPEC27@{ shape: rect, label: "CAPEC-27: Leveraging Race Conditions" }
CAPEC25@{ shape: rect, label: "CAPEC-25: Forced Deadlock" }
end
subgraph SL4["4. Threats"]
direction LR
S1@{ shape: rect, label: "STRIDE-1: Hook Directory Takeover<br><i>High / Possible</i>" }
S6@{ shape: rect, label: "STRIDE-6: Ownership Check Race<br><i>Medium / Possible</i>" }
end
subgraph SL5["5. Threat Actors"]
direction LR
TA2@{ shape: rect, label: "TA-2: Local Multi-Tenant Attacker<br><i>Escalate privileges via shared hook install race</i>" }
end
SC1 --> CWE367
SC1 --> CWE362
CWE367 --> CAPEC27
CWE362 --> CAPEC25
CAPEC27 --> S1
CAPEC25 --> S6
S1 --> TA2
S6 --> TA2
linkStyle 0 stroke:#FF0000,stroke-width:2px
linkStyle 1 stroke:#FFA500,stroke-width:2px
linkStyle 2 stroke:#FF0000,stroke-width:2px
linkStyle 3 stroke:#FFA500,stroke-width:2px
linkStyle 4 stroke:#FF0000,stroke-width:2px
linkStyle 5 stroke:#FFA500,stroke-width:2px
linkStyle 6 stroke:#FF0000,stroke-width:2px
linkStyle 7 stroke:#FFA500,stroke-width:2px
SC-2: Generated Git Hook Dispatcher
---
config:
layout: dagre
look: classic
theme: dark
---
flowchart LR
subgraph SL1["1. System Component"]
direction LR
SC2@{ shape: rect, label: "SC-2: Generated Git Hook Dispatcher" }
end
subgraph SL2["2. Weaknesses"]
direction LR
CWE354@{ shape: rect, label: "CWE-354: Improper Validation of Integrity Check" }
CWE20@{ shape: rect, label: "CWE-20: Improper Input Validation" }
CWE668@{ shape: rect, label: "CWE-668: Exposure of Resource to Wrong Sphere" }
end
subgraph SL3["3. Attack Patterns"]
direction LR
CAPEC668@{ shape: rect, label: "CAPEC-668: Key Negotiation of Bluetooth Attack (bypass analogue)" }
CAPEC267@{ shape: rect, label: "CAPEC-267: Leverage Alternate Encoding" }
CAPEC176@{ shape: rect, label: "CAPEC-176: Configuration/Environment Manipulation" }
end
subgraph SL4["4. Threats"]
direction LR
S3@{ shape: rect, label: "STRIDE-3: --no-verify Bypass<br><i>High / Likely</i>" }
S2@{ shape: rect, label: "STRIDE-2: DID Trailer Injection<br><i>Medium / Possible</i>" }
S9@{ shape: rect, label: "STRIDE-9: Global Cross-Repo Contamination<br><i>High / Likely</i>" }
end
subgraph SL5["5. Threat Actors"]
direction LR
TA1@{ shape: rect, label: "TA-1: Malicious Insider Contributor<br><i>Evade non-repudiation</i>" }
TA3@{ shape: rect, label: "TA-3: Malicious Repository Publisher<br><i>Trick victim into signing under wrong persona</i>" }
end
SC2 --> CWE354
SC2 --> CWE20
SC2 --> CWE668
CWE354 --> CAPEC668
CWE20 --> CAPEC267
CWE668 --> CAPEC176
CAPEC668 --> S3
CAPEC267 --> S2
CAPEC176 --> S9
S3 --> TA1
S2 --> TA1
S9 --> TA3
linkStyle 0 stroke:#FF0000,stroke-width:2px
linkStyle 1 stroke:#FFA500,stroke-width:2px
linkStyle 2 stroke:#FF0000,stroke-width:2px
linkStyle 3 stroke:#FF0000,stroke-width:2px
linkStyle 4 stroke:#FFA500,stroke-width:2px
linkStyle 5 stroke:#FF0000,stroke-width:2px
linkStyle 6 stroke:#FF0000,stroke-width:2px
linkStyle 7 stroke:#FFA500,stroke-width:2px
linkStyle 8 stroke:#FF0000,stroke-width:2px
linkStyle 9 stroke:#FF0000,stroke-width:2px
linkStyle 10 stroke:#FFA500,stroke-width:2px
linkStyle 11 stroke:#FF0000,stroke-width:2px
SC-3: verify-trust CI Verifier
---
config:
layout: dagre
look: classic
theme: dark
---
flowchart LR
subgraph SL1["1. System Component"]
direction LR
SC3@{ shape: rect, label: "SC-3: verify-trust CI Verifier" }
end
subgraph SL2["2. Weaknesses"]
direction LR
CWE706@{ shape: rect, label: "CWE-706: Use of Incorrectly-Resolved Name/Reference" }
CWE346@{ shape: rect, label: "CWE-346: Origin Validation Error" }
end
subgraph SL3["3. Attack Patterns"]
direction LR
CAPEC141@{ shape: rect, label: "CAPEC-141: Cache Poisoning (fallback analogue)" }
CAPEC151@{ shape: rect, label: "CAPEC-151: Identity Spoofing" }
end
subgraph SL4["4. Threats"]
direction LR
S4@{ shape: rect, label: "STRIDE-4: Legacy Fallback Downgrade<br><i>Medium / Possible</i>" }
end
subgraph SL5["5. Threat Actors"]
direction LR
TA1b@{ shape: rect, label: "TA-1: Malicious Insider Contributor<br><i>Forge attribution via weaker legacy path</i>" }
end
SC3 --> CWE706
SC3 --> CWE346
CWE706 --> CAPEC141
CWE346 --> CAPEC151
CAPEC141 --> S4
CAPEC151 --> S4
S4 --> TA1b
linkStyle 0 stroke:#FFA500,stroke-width:2px
linkStyle 1 stroke:#FFA500,stroke-width:2px
linkStyle 2 stroke:#FFA500,stroke-width:2px
linkStyle 3 stroke:#FFA500,stroke-width:2px
linkStyle 4 stroke:#FFA500,stroke-width:2px
linkStyle 5 stroke:#FFA500,stroke-width:2px
linkStyle 6 stroke:#FFA500,stroke-width:2px
SC-4: Local Git Config Store
---
config:
layout: dagre
look: classic
theme: dark
---
flowchart LR
subgraph SL1["1. System Component"]
direction LR
SC4@{ shape: rect, label: "SC-4: Local Git Config Store" }
end
subgraph SL2["2. Weaknesses"]
direction LR
CWE459@{ shape: rect, label: "CWE-459: Incomplete Cleanup" }
CWE59@{ shape: rect, label: "CWE-59: Improper Link Resolution" }
CWE755@{ shape: rect, label: "CWE-755: Improper Handling of Exceptional Conditions" }
end
subgraph SL3["3. Attack Patterns"]
direction LR
CAPEC462@{ shape: rect, label: "CAPEC-462: Cross-Domain Search-Path Alteration" }
CAPEC132@{ shape: rect, label: "CAPEC-132: Symlink Attack" }
CAPEC548@{ shape: rect, label: "CAPEC-548: Contaminate Resource" }
end
subgraph SL4["4. Threats"]
direction LR
S8@{ shape: rect, label: "STRIDE-8: Stale hooksPath after uninstall<br><i>Low / Possible</i>" }
S5@{ shape: rect, label: "STRIDE-5: Symlink Path Traversal<br><i>Medium / Unlikely</i>" }
S7@{ shape: rect, label: "STRIDE-7: Non-Fatal Install Failure<br><i>Medium / Likely</i>" }
end
subgraph SL5["5. Threat Actors"]
direction LR
TA2b@{ shape: rect, label: "TA-2: Local Multi-Tenant Attacker<br><i>Exploit stale/dangling config state</i>" }
TA3b@{ shape: rect, label: "TA-3: Malicious Repository Publisher<br><i>Craft repo to redirect hook install path</i>" }
end
SC4 --> CWE459
SC4 --> CWE59
SC4 --> CWE755
CWE459 --> CAPEC462
CWE59 --> CAPEC132
CWE755 --> CAPEC548
CAPEC462 --> S8
CAPEC132 --> S5
CAPEC548 --> S7
S8 --> TA2b
S5 --> TA3b
S7 --> TA2b
linkStyle 0 stroke:#00FF00,stroke-width:2px
linkStyle 1 stroke:#FFA500,stroke-width:2px
linkStyle 2 stroke:#FFA500,stroke-width:2px
linkStyle 3 stroke:#00FF00,stroke-width:2px
linkStyle 4 stroke:#FFA500,stroke-width:2px
linkStyle 5 stroke:#FFA500,stroke-width:2px
linkStyle 6 stroke:#00FF00,stroke-width:2px
linkStyle 7 stroke:#FFA500,stroke-width:2px
linkStyle 8 stroke:#FFA500,stroke-width:2px
linkStyle 9 stroke:#00FF00,stroke-width:2px
linkStyle 10 stroke:#FFA500,stroke-width:2px
linkStyle 11 stroke:#FFA500,stroke-width:2px
📊 Risk Summary
Total Threats: 10
By Severity: Low: 1 · High: 3 · Medium: 6
By Category: Tampering: 10 · Elevation of Privilege: 3 · Spoofing: 3 · Repudiation: 2 · Denial of Service: 3 · Information Disclosure: 2
🎯 Attack Surface
Kill Chain 1: A local multi-tenant attacker (TA-2) on a shared build or development machine exploits the non-atomic sequence in install_hook_dispatcher (STRIDE-1) — directory creation, then per-hook file writes, then finally the core.hooksPath config write — to plant a malicious hook file that survives the ownership marker check, or races the ownership check itself (STRIDE-6) against a concurrently-installing tool like husky; once core.hooksPath is committed, every subsequent git operation on that machine executes attacker code with the victim's privileges, providing a path to exfiltrate the Ed25519 signing key (RA-1) directly from process memory before its zeroization completes. Kill Chain 2: A malicious insider contributor (TA-1) with legitimate commit access chains the client-side-only enforcement gap (STRIDE-3, git commit --no-verify) with the dual-path DID discovery weakness in verify-trust (STRIDE-4) — producing a validly-signed commit with no Signed-by-DID trailer, then relying on either a permissive legacy user.email fallback or an imperfect CI gate to have it merged; because the trailer is the sole non-repudiation anchor for modern commits, this chain launders code changes into the trusted history while preserving deniability for the actual author. Kill Chain 3: A malicious repository publisher (TA-3) crafts a git repository or worktree structure that manipulates git rev-parse --absolute-git-dir output (STRIDE-5) to redirect where did-git-sign init writes its hook dispatcher, combined with the global installation cross-contamination weakness (STRIDE-9) — a victim who previously ran --global init and later clones this repository has their DID identity and signing key silently exposed to the untrusted repository's execution context, since the delegating hooks transparently chain to any .git/hooks scripts the malicious repository provisions via its own setup tooling. Kill Chain 4: Any of the above chains is compounded by the non-fatal error handling in install() (STRIDE-7) — if hook installation fails partway due to any of the above conflicts, the overall init command still reports success, meaning automation and CI bootstrap scripts that check only the exit code will proceed with the false assumption that DID-trailer enforcement (RA-3) is active, extending the window during which unattributed or attacker-manipulated commits can accumulate before CI's verify-trust check ultimately catches (or fails to catch) the gap.
🛡️ Risk Mitigation Strategy
Priority 1 (Immediate): The most severe systemic gap is that DID-claim enforcement is entirely client-side and trivially bypassable via git commit --no-verify (STRIDE-3), with a compensating server-side control absent from the reviewed artifacts. This single gap undermines every other control in the system, since a non-repudiation architecture that can be silently disabled by the committer it is meant to hold accountable provides no real guarantee. Server-side pre-receive hook enforcement of the Signed-by-DID trailer, paired with branch protection requiring the noSignerDid CI check with no override for standard contributors, must be implemented before this tool can be relied upon for supply-chain-critical repositories. Priority 2 (Short-Term): The hook installation lifecycle (STRIDE-1, STRIDE-6, STRIDE-9) exhibits classic TOCTOU and shared-resource-contention patterns across three related but distinct threats — non-atomic directory population, unlocked core.hooksPath ownership checks, and unscoped global identity binding. These should be addressed together via an install-time redesign: populate hook contents in a temporary directory with restrictive permissions, atomically rename into place, re-verify core.hooksPath ownership immediately before the final config write, and default to local-only installation with global scope requiring explicit, loudly-surfaced confirmation given its cross-repository identity-binding consequences. Priority 3 (Medium-Term): Input validation on the DID value throughout the commit-msg hook (STRIDE-2, STRIDE-10) currently relies on blocklist-style whitespace and prefix checks rather than a strict grammar, creating a narrow but real trailer-confusion and log-injection surface; verify-trust should independently and strictly validate DID syntax rather than trusting hook-side sanitization, and the hook itself should move to an allow-list character model. Priority 4 (Medium-Term): Observability gaps — non-fatal install failures reported only v
Generated by Agentic Sec — Threat Model & Affect Analysis Agent
📊 Summary & findings
| ✅ Confirmed | |
|---|---|
| 1 | 0 |
Confirmed (1)
- 🟡 TOCTOU race between core.hooksPath check and write in install_hook_dispatcher (triaged HIGH→MEDIUM)
🔄 Security Rescan Update
📊 2 confirmed issues currently open on this change. 📌 See the pinned Security Review comment for full details and reports. 🔄 Automated Security Rescan Update |
Signed-off-by: Robert Kwolek <robert.k@affinidi.com> Signed-by-DID: did:webvh:QmNYECKwYUJExB19ucYGwRjvRGyPPk5ShGLUkxjiLLyyVm:affinidi.github.io:did-docs:robert#key-0
🔄 Security Rescan Update
📊 1 confirmed issue currently open on this change. 📌 See the pinned Security Review comment for full details and reports. 🔄 Automated Security Rescan Update |
🔄 Security Rescan Update
📊 2 confirmed issues currently open on this change. 📌 See the pinned Security Review comment for full details and reports. 🔄 Automated Security Rescan Update |
…able Review fixes for the trailer flow. The hook wrote the trailer in a way neither Linux nor git could handle, and taking core.hooksPath could silently disable a repository's existing hooks. - commit-msg hook: delegate trailer placement to `git interpret-trailers`. `sed -i ''` is BSD-only — GNU sed reads the empty argument as a filename, fails, and leaves the file unchanged, so the strip loop that re-tested the same condition spun forever and `git commit` hung on Linux. And on a one-line message (`git commit -m fix`) the trailer was appended straight onto the subject, where git's own parser reads no trailers at all: `git log --format=%(trailers)`, forges and DCO tooling all saw nothing. - commit-msg hook: read `DID_GIT_SIGN_KEY` before `did-git-sign.key`, the same precedence the signer uses. Otherwise `DID_GIT_SIGN_KEY=… git commit` wrote one DID into the trailer, signed with another, and refused its own commit. - commit-msg hook: make `Signed-off-by:` opt-in via `did-git-sign.signoff`. A DCO sign-off asserts something about the committer's right to submit the code; a signing tool must not assert it on their behalf. - init: refuse to take `core.hooksPath` in *both* scopes. Only `--global` was guarded, so a local install silently clobbered husky/lefthook/ pre-commit — the delegating hooks fall back to `$git_dir/hooks`, never to the path that was configured before, so those hooks just stopped running. - init: write the hook directory before pointing `core.hooksPath` at it. A hook write can fail partway, and the config previously already named a half-populated directory while the caller swallowed the error as a warning. - init: replace `to_str().unwrap()` with a real error, and say in the best-effort warning that signing *stops* without the hook rather than merely losing a trailer. - sign: collapse `check_committer_matches_key` onto `signer_did`. Its trailing `match committer_did(...)` had become unreachable for both `Some(_)` arms — `signer_did` already falls back to `committer_did` — so a mismatched legacy committer got the generic message instead of the one naming the fix. - Cover the hook with tests that run it, not tests that grep its source. Both bugs above passed every `contains` assertion that existed. - README/RUNBOOK: document the trailer, `did-git-sign.key` as the single identity selector, and the hook dispatcher. They still described `user.email = <DID#key-id>` as load-bearing and told users to fix `noSignerDid` by re-running init to set it. - Fix five clippy lints failing CI. Claude-Session: https://claude.ai/code/session_01H8m2hRAbLeAms1aHnT2bJe Signed-off-by: Glenn Gore <glenn.g@affinidi.com>
3 participants
add Signed-by-DID trailer for git-host email compatibility
Move the signer DID claim from user.email into a Signed-by-DID git trailer
injected by a commit-msg hook. user.email stays a normal email address so
GitLab/GitHub can attribute commits to accounts.
instead of overwriting user.email
present; refuse when neither exists (missing hook)