Aman-Mittal opened a new pull request, #430: URL: https://github.com/apache/fineract-backoffice-ui/pull/430
Closes #428. Two workflows: the signed-commit gate now asks GitHub whether it verified each commit and explains on the PR when it did not, and first-time contributors get a greeting with the code of conduct, the community channels and the requirements that most often block a first change. ## The check was asking the wrong question `verify-signed-commits.sh` passed anything whose `%G?` was not `N` — "is a signature attached?". GitHub asks whether the signature was made by a key registered to an account whose verified email matches the committer. Commits pass the first and fail the second routinely: | PR | check | commits | verified | reason | |---|---|---|---|---| | #392 | ✅ success | 3 | 0 | `no_user` | | #331 | ✅ success | 2 | 0 | `no_user` | A runner cannot answer the stricter question — it holds no contributor public keys, which is exactly why the old script documented `E`/`U`/`B` as deliberate passes. It does not need to: GitHub already computed the answer and returns it per commit as `commit.verification`. ## The gate could be edited by the branch it gates Under `pull_request`, the workflow *and* its scripts come from the merge ref — the PR's own copy. A contributor can therefore weaken the check meant to gate them, which is not hypothetical: it is how the accept-any-`gpgsig` fallback in #392 came about. `pull_request_target` runs the base repository's copy and carries a token that can comment, which a fork's `pull_request` run deliberately does not (read-only token, so the current workflow could not comment even if it wanted to). **What keeps that safe:** neither workflow checks out or executes the PR's code. Both take a ref-less, sparse checkout of `.github/scripts` from the base branch and read only the API response — never the diff, never the commit tree. Commit *subjects* do reach the comment body, so they are truncated and their `@`/`#` references defused. This follows the reasoning already written down in `pr-comments.yml`. zizmor passes at this repository's own settings (`--min-severity informational --min-confidence low`); the two `dangerous-triggers` findings are ignored inline with justification, matching the existing convention. ## Failures explain themselves Grouped by `verification.reason`, because the remedies are not interchangeable — which is why a blanket "please sign your commits" has such a poor hit rate. `no_user`, the reason on both affected PRs, is **not a signing problem**: the signature is fine and `user.email` is not an address on any GitHub account. Telling those contributors to set up signing sends them to redo the one part they got right. Rendered from the live API against #392: > ### `no_user` > > These commits **are signed**, but the committer email is not an address GitHub can match to any account — so it never gets as far as checking the key. Note that the commits are not attributed to your GitHub profile either, which is the same cause. > > - `9d7f1793` feat(clients): add empty-state actions on account tabs > - `74f75234` Merge upstream main and resolve client-view spec conflict > - `af3ef580` ci: recognise SSH commit signatures on the Actions runner > > **How to fix:** This is an identity problem, not a signing one — your signing setup is fine. Check `git config user.email` and set it to an address that is **verified on your GitHub account** (Settings → Emails), then re-sign. One comment per PR, updated on push, **deleted** once every commit verifies so a fixed PR keeps no stale failure. `scripts/verify-signed-commits.sh` stays as a local pre-push aid, with a header stating plainly that it is no longer the gate and explaining why it cannot be one. ## Welcome workflow First-time contributors (`FIRST_TIME_CONTRIBUTOR` / `FIRST_TIMER`, straight from the payload — no API call, no rate limit) get the [ASF Code of Conduct](https://www.apache.org/foundation/policies/conduct), the [dev mailing list](https://lists.apache.org/[email protected]) with the `dev-subscribe@` address, the [Matrix rooms](https://matrix.to/#/%23apache-fineract-home:matrix.org), and the six requirements that most often block a first PR. Bots skipped; idempotent on reopen. It is deliberately short. CONTRIBUTING.md and the PR template already state everything, and they still get missed — restating all of it would repeat something that demonstrably does not land. ## Testing Neither workflow can be exercised by the PR that adds it — `pull_request_target` only ever runs the base branch's copy — so **the unit tests are the entire pre-merge safety net**. `scripts/pr-comment-actions.test.mjs` (16 new tests, `npm run test:scripts`) covers comment idempotency, deletion on success, never overwriting a human comment who happens to quote the marker, per-reason grouping, and defusing `@`/`#` in attacker-controlled subjects. Both scripts were also run against the live GitHub API for #392 (fails, `no_user`) and #393 (passes, no comment). That is what caught a rendering bug the unit tests missed: `.filter(Boolean)` was stripping the intentional blank lines out of the body, so headings welded to the paragraph above them. The string still "worked" and every assertion still passed — only reading the posted output showed it. There is now a test pinning it. All 13 static checks, `test:scripts` (28 passing) and `check-license.sh` pass. ## Still worth doing separately Enable branch protection **Require signed commits** on `main`. GitHub enforces that itself and no workflow can weaken it. This check is the explanation, not the wall — and #428 has the rest of the reasoning. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
