hyperledger-identus / hyperledger-identus/sdk-rust

fix(factory): validate rebased PR commits against current develop

Open
#333 0 comments 0 reactions 0 assignees View on GitHub
enhancement
Dominant language
Rust
Stars
0
Forks
0
Avg merge
39m
Merged PRs (30d)
153

Description

## Problem

The repository pre-push hook validates `remoteSha..localSha`. After an open PR
branch is rebased onto a newly advanced `origin/develop`, the old remote branch
head is no longer an ancestor of the local head. `git rev-list
remoteSha..localSha` then includes the newly merged `develop` squash commits as
well as the rebased PR commits.

Those protected squash commits carry valid GitHub OpenPGP signatures, but the
local keyring does not contain GitHub's web-flow public key. The hook therefore
fails with `local OpenPGP cryptographic verification failed` for trusted base
commits outside the pull request. This blocks the normal strict-up-to-date
rebase workflow even when every rebased PR commit verifies locally and GitHub
reports the base commit signature as valid.

Observed while rebasing PR #332 after PR #331 advanced `develop`:

- remote PR head: `f81ab610f48ab2ef74550fe5adb34ca7f2b1fc20`
- new protected base: `bd79ace91e4ddf5049cd5e03757e7db03eb3dd3f`
- the hook attempted local verification of the protected base squash commit
and rejected it
- all four rebased PR commits passed `git verify-commit`; GitHub reported the
base commit `verification.verified=true`, reason `valid`

## Acceptance criteria

- [ ] Pre-push derives the contribution range from the merge base with current
`origin/develop` when the remote branch head is not an ancestor of the
local head.
- [ ] Only commits introduced by the feature branch require local DCO/OpenPGP
verification.
- [ ] A normal fast-forward update still validates only newly pushed commits.
- [ ] A force-with-lease rebase onto protected `develop` succeeds when all
feature commits verify and fails when any feature commit is unsigned.
- [ ] Base commits are never silently trusted from an arbitrary ref; the base
must resolve to current fetched `origin/develop` and be an ancestor of
the local head.
- [ ] Tests cover divergent old remote head, malicious unrelated history,
missing base, and ordinary first push.

## Scope

Factory/hook correctness only. Do not weaken hosted PR policy, DCO, signature,
branch naming, or exact-head merge enforcement.

Contributor guide

Open the contributing guide

Research direction

Start by locating the Factory pre-push hook and its existing tests, then trace how it derives the commit range and performs DCO/OpenPGP verification. Exercise the hook with the divergent old remote head, missing or unrelated bases, and an ordinary first push. Done means only feature commits are checked, current origin/develop is required as an ancestor, and the specified rebase and unsigned-commit cases pass.

Written by the indexing model from the issue text.

Assessment

Tech stack
git, rust
Domain
security, testing-qa, tooling
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.