Conversation
WalkthroughAdds three Husky Git hooks: Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant Git as Git CLI
participant CM as .husky/commit-msg
participant PC as .husky/post-commit
participant PP as .husky/pre-push
Dev->>Git: git commit
Git->>CM: run commit-msg
alt Missing "Signed-off-by:"
CM-->>Git: exit 1 (abort)
Git-->>Dev: show guidance
else Has "Signed-off-by:"
CM-->>Git: exit 0
Git->>PC: run post-commit
alt Merge commit
PC-->>Git: skip (exit 0)
else Non-merge
PC->>Git: check GPG verification
alt Verified
PC-->>Git: exit 0
else Unverified
PC-->>Git: exit 1 (report)
end
end
end
Dev->>Git: git push
Git->>PP: run pre-push
PP->>Git: determine push range (from..to)
loop each non-merge commit in range
PP->>Git: check GPG signature
end
alt Any unsigned commits
PP-->>Dev: offer auto-sign (rebase/exec) or manual steps
PP-->>Git: exit 1 (block push)
else All signed
PP-->>Git: exit 0 (allow push)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
49773a6 to
47b64d3
Compare
Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>
Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>
Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>
Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>
Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>
Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>
Signed-off-by: sujitaw <sujit.sutar@ayanworks.com> Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>
Signed-off-by: sujitaw <sujit.sutar@ayanworks.com> Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>
Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>
Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>
Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>
Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>
Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>
af849dd to
11adc77
Compare
Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>
Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (11)
README.md (1)
116-123: Document the new commit-signing enforcement in Contributing/README.Add a short note that Husky hooks enforce DCO “Signed-off-by” and cryptographic signatures, plus how to install hooks (npm run prepare) and how to bypass in CI if needed.
.husky/commit-msg (1)
1-9: Harden DCO check (anchor, email shape) and align policy for merges.
- Current grep can match false positives; anchor and validate trailer format.
- Consider skipping merge commits for consistency with post-commit (which skips merges).
- Add safety flags for robustness.
#!/usr/bin/env sh -. "$(dirname -- "$0")/_/husky.sh" +. "$(dirname -- "$0")/_/husky.sh" +set -eu -commit_msg_file=$1 -if ! grep -q "Signed-off-by:" "$commit_msg_file"; then +commit_msg_file=${1:-} +if [ -z "${commit_msg_file}" ] || [ ! -f "${commit_msg_file}" ]; then + printf '%s\n' "Commit message file missing"; exit 1 +fi + +# Optional: skip merges (aligns with post-commit behavior) +if git rev-parse -q --verify MERGE_HEAD >/dev/null 2>&1; then + exit 0 +fi + +# Strict DCO trailer: case-sensitive label, anchored, with email in <> +if ! grep -Eq '^Signed-off-by:[[:space:]].+<[^@<>[:space:]]+@[^@<>[:space:]]+>$' "$commit_msg_file"; then echo "Commit message missing Signed-off-by line" echo "Use: git commit -s ..." exit 1 fi.husky/post-commit (3)
1-8: Fix color output for POSIX sh and add safety flags.Variables like "\033" aren’t interpreted in sh; use printf to build colors and set -eu.
#!/usr/bin/env sh -RESET="\033[0m" -RED="\033[0;31m" -GREEN="\033[0;32m" -YELLOW="\033[0;33m" -BLUE="\033[0;34m" -CYAN="\033[0;36m" +set -eu +RESET=$(printf '\033[0m') +RED=$(printf '\033[0;31m') +GREEN=$(printf '\033[0;32m') +YELLOW=$(printf '\033[0;33m') +BLUE=$(printf '\033[0;34m') +CYAN=$(printf '\033[0;36m')
23-31: Accept “U” (good but untrusted) signatures too.Git’s %G? can be G or U for valid signatures; treating only G as valid will incorrectly fail many users.
-sig_status=$(git log --format='%G?' -n 1 $last_commit) +sig_status=$(git log --format='%G?' -n 1 "$last_commit") -if [ "$sig_status" != "G" ]; then +if [ "$sig_status" != "G" ] && [ "$sig_status" != "U" ]; then
38-41: Post-commit cannot block commits; consider warn-only.Git ignores post-commit’s exit code. Prefer exit 0 with guidance to avoid confusion; enforcement happens in pre-push or server-side.
.husky/pre-push (6)
1-11: Make colors portable and add safety flags.Use printf-built colors and set -eu for safer execution.
#!/usr/bin/env sh -. "$(dirname -- "$0")/_/husky.sh" +. "$(dirname -- "$0")/_/husky.sh" +set -eu -# ANSI colors -RED="\033[0;31m" -YELLOW="\033[0;33m" -CYAN="\033[0;36m" -GREEN="\033[0;32m" -RESET="\033[0m" +# ANSI colors +RED=$(printf '\033[0;31m') +YELLOW=$(printf '\033[0;33m') +CYAN=$(printf '\033[0;36m') +GREEN=$(printf '\033[0;32m') +RESET=$(printf '\033[0m')
32-36: Treat “U” as valid signature.Avoid false negatives by accepting good-but-untrusted (%G? == U).
- sig_status=$(git log --format='%G?' -n 1 $commit) + sig_status=$(git log --format='%G?' -n 1 "$commit") - if [ "$sig_status" != "G" ]; then + if [ "$sig_status" != "G" ] && [ "$sig_status" != "U" ]; then unsigned_commits="$unsigned_commits $commit" fi
43-46: Don’t prompt in non-interactive environments.Pre-push often runs in non-interactive shells; reading from /dev/tty can hang. Gate prompts on TTY; otherwise, print instructions and exit 1.
- echo "${YELLOW}Do you want to automatically sign these commits? [y/N]${RESET}" - read -r answer </dev/tty + if [ -t 0 ] && [ -t 1 ]; then + echo "${YELLOW}Do you want to automatically sign these commits? [y/N]${RESET}" + read -r answer </dev/tty + else + answer="N" + fi
65-73: Manual guidance: preserve merges and handle root-commit.Suggest --rebase-merges and a root-safe variant.
- echo " - For the earliest unsigned commit $short_hash:" - echo " git rebase -i <base_branch> --onto $earliest_commit^ --exec 'git commit --amend -sS --no-edit'" + echo " - For the earliest unsigned commit $short_hash (preserve merges):" + echo " # If $earliest_commit has a parent" + echo " git rebase -i --rebase-merges $earliest_commit^ --exec 'git commit --amend -sS --no-edit'" + echo " # If it's the root commit:" + echo " git rebase -i --rebase-merges --root --exec 'git commit --amend -sS --no-edit'"
15-19: Initial push range can be large; consider limiting to commits reachable from the branch tip.Optional: use rev-list with --first-parent to reduce traversal on long histories.
- range="$local_sha" + range="$(git rev-list --first-parent "$local_sha")"
86-86: Minor: message says “signed and verified” but logic treats U as valid.If you accept U, consider “signed” or “signed (G/U)” to avoid confusion.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.husky/commit-msg(1 hunks).husky/post-commit(1 hunks).husky/pre-push(1 hunks)README.md(1 hunks)
Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
.husky/commit-msg (3)
6-11: Send warnings/errors to stderr for cleaner logs and CI signal.Route non-happy-path messages to stderr.
- echo "⚠️ No commit message file provided or file does not exist." - echo "👉 Skipping DCO sign-off check." + echo "⚠️ No commit message file provided or file does not exist." >&2 + echo "👉 Skipping DCO sign-off check." >&2 @@ - echo "Commit message missing Signed-off-by line" - echo "Use: git commit -s ..." + echo "Commit message missing Signed-off-by line" >&2 + echo "Use: git commit -s ..." >&2Also applies to: 14-16
1-5: Harden shell: enable strict mode and safe positional param expansion.Prevents silent failures if $1 is unset.
#!/usr/bin/env sh . "$(dirname -- "$0")/_/husky.sh" -commit_msg_file=$1 +set -eu +commit_msg_file=${1-}
13-17: Use git’s trailer parser + anchored, case‑insensitive match for robust DCO detection.Avoids false positives in the body and validates trailer shape.
-if ! grep -q "Signed-off-by:" "$commit_msg_file"; then - echo "Commit message missing Signed-off-by line" - echo "Use: git commit -s ..." - exit 1 -fi +if ! git -c trailer.separators=: interpret-trailers --parse < "$commit_msg_file" \ + | grep -qiE '^Signed-off-by:[[:space:]]+[^<]+<[^@<>[:space:]]+@[^@<>[:space:]]+>[[:space:]]*$' +then + echo "Commit message missing a valid 'Signed-off-by: Name <email>' trailer." >&2 + echo "Fix this commit: git commit --amend -s --no-edit" >&2 + echo "Create new commits with sign-off: git commit -s ..." >&2 + exit 1 +fiFallback (if interpret-trailers is unavailable): replace the condition with
grep -qiE '^[[:space:]]*Signed-off-by:[[:space:]]+[^<]+<[^>]+>[[:space:]]*$' "$commit_msg_file" || exit 1.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.husky/commit-msg(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: GHkrishna
PR: credebl/platform#1468
File: .husky/pre-push:47-56
Timestamp: 2025-09-29T12:14:52.466Z
Learning: User GHkrishna prefers allowing automatic rebase in pre-push hooks for signing commits, considering it acceptable since they're rebasing their own local commits before sharing.
🔇 Additional comments (2)
.husky/commit-msg (2)
6-11: Good guard for empty/missing commit message file.Addresses the prior feedback to avoid grepping an empty path. Nice.
1-1: No changes required — .husky/commit-msg is already executable.
Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>
|
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
.husky/pre-push (2)
24-31: Simplify merge skipping using--no-merges(faster, less plumbing).You can avoid extra subprocesses and the skip message logic.
- for commit in $(git rev-list $range); do - # Skip merge commits - parent_count=$(git rev-list --parents -n 1 $commit | awk '{print NF-1}') - if [ "$parent_count" -gt 1 ]; then - echo "${CYAN}ℹ️ Skipping merge commit $commit${RESET}" - continue - fi + for commit in $(git rev-list --no-merges "$range"); doIf you want to keep the informational log, you can print a single summary count of merges skipped.
4-11: Minor: Use printf for colors to ensure escape handling in POSIX sh.
echodoesn’t reliably interpret\033. Preferprintf '%b\n'.-RED="\033[0;31m" -YELLOW="\033[0;33m" -CYAN="\033[0;36m" -GREEN="\033[0;32m" -RESET="\033[0m" +RED="$(printf '\033[0;31m')" +YELLOW="$(printf '\033[0;33m')" +CYAN="$(printf '\033[0;36m')" +GREEN="$(printf '\033[0;32m')" +RESET="$(printf '\033[0m')"Alternatively, define a helper:
sayc() { printf '%b\n' "$1$2$RESET"; }.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.husky/pre-push(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: GHkrishna
PR: credebl/platform#1468
File: .husky/pre-push:47-56
Timestamp: 2025-09-29T12:14:52.466Z
Learning: User GHkrishna prefers allowing automatic rebase in pre-push hooks for signing commits, considering it acceptable since they're rebasing their own local commits before sharing.
📚 Learning: 2025-09-29T12:14:52.466Z
Learnt from: GHkrishna
PR: credebl/platform#1468
File: .husky/pre-push:47-56
Timestamp: 2025-09-29T12:14:52.466Z
Learning: User GHkrishna prefers allowing automatic rebase in pre-push hooks for signing commits, considering it acceptable since they're rebasing their own local commits before sharing.
Applied to files:
.husky/pre-push
* fix: something random Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com> * fix: add push DCO protection Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com> * added space Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com> * fix: dco script Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com> * fix: something random Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com> * fix: pre-commit and pre-push hooks for signoff Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com> * Test pre commit Signed-off-by: sujitaw <sujit.sutar@ayanworks.com> Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com> * Test no verify Signed-off-by: sujitaw <sujit.sutar@ayanworks.com> Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com> * Test no verify Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com> * test Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com> * test Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com> * test Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com> * test Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com> * fix: final changes with informative logging Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com> * fix: auto signoff Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com> * fix: unwanted file removed Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com> * fix: pre pus Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com> * fix: remove unwanted code Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com> * fix: remove commensts Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com> * fix: defensively handle pre-commits Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com> * fix: avoid flattening merge commits Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com> --------- Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com> Signed-off-by: sujitaw <sujit.sutar@ayanworks.com> Co-authored-by: pranalidhanavade <pranali.dhanavade@ayanworks.com> Co-authored-by: Sai Ranjit Tummalapalli <sairanjit.tummalapalli@ayanworks.com> Co-authored-by: sujitaw <sujit.sutar@ayanworks.com> Signed-off-by: Ankita Patidar <ankita.patidar@ayanworks.com>



What
Summary by CodeRabbit