fix(ci): use HTTP basic auth for git token push - #43
Conversation
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Warning Review limit reachedNext included review available in 34 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Repository: comarch/git-byline/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (6)
Summary by CodeRabbit
WalkthroughChangesGitHub Git fetch, notes, push, and release tag operations now use Basic authentication with an GitHub Basic authentication
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix Merge Risk: 🟠 High · up to Attribution-note pushes can fail whenever notes exist, and workflow regeneration may reintroduce an outdated Go version. Fix both before merging. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. (5 skipped: 5 unsupported.) ✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: comarch/git-byline/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Advanced
Run ID: 6b703b65-efb7-4c86-8598-5aa5015b9aa1
📒 Files selected for processing (6)
.github/workflows/git-byline.yml.github/workflows/release.ymlaction.ymlaction/action.ymlinternal/ci/ci_test.gointernal/ci/templates/github.yml
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.
GitHub git endpoints reject Authorization: bearer with 401. The v1.4.0 release run published all artifacts, then the new action major tag step failed with 'could not read Username' and re-runs died on 422 already_exists. The forge notes push had the same latent scheme bug. Send the token as x-access-token over HTTP basic, matching how actions/checkout authenticates git traffic.
e4e5b98 to
620cc6e
Compare
|
* fix(ci): install pinned release binary in forge workflows Both templates ran go run ./cmd/git-byline, which works only in this repo. In any other project the reconstruct job failed on this step. Now the workflow downloads the release pinned on the GIT_BYLINE_VERSION line, checks the archive against checksums.txt and the binary version, same like action.yml and install.sh, and runs that binary. Release Please bumps the pin in both templates, and validate fails when the pin and the manifest drift. A workflow that differs from the template only in the pinned tag still counts as managed, so an upgrade does not make install-hooks warn or uninstall skip the file. The GitLab job runs in buildpack-deps:trixie-scm now, it does not need Go anymore. GIT_BYLINE_VERSION and GIT_BYLINE_RELEASES_URL can be set as CI/CD variables, for example for an internal mirror. The GitHub workflow triggers on the default branch instead of hardcoded main. Fixes #50 * fix(ci): use HTTP basic auth in GitLab forge job The job put GITLAB_TOKEN into http.extraHeader as PRIVATE-TOKEN. This header is for REST calls, Git over HTTPS takes HTTP basic auth. With credential.helper empty Git had no other way to log in, so the first authenticated fetch or the notes push got 401. Now the header is basic auth with user oauth2 and the token as password, same idea like #43 for GitHub. I checked in the runner image that Git sends it on the first request and that it decodes to oauth2:<token>. A run against live GitLab is still to verify in the pilot. A missing GITLAB_TOKEN also fails with a message now, not a bare test. Fixes #51 * fix(ci): reconstruct GitLab squash merges from merge request head After a squash merge the MR commits, and their notes, stay only on refs/merge-requests/<iid>/head. The job took the source from commit parents, so a squash with fast-forward had an empty source range and a squash with merge commit pointed at the squash commit, which has no notes. In both cases the lines stayed untracked. Now the job reads the <project path>!<iid> reference from the merged commit message, fetches the MR head and uses it as the source, but only when its diff has the same stable patch ID as the diff the target commit brings. Without the reference it keeps the parents like before. When the diff differs it keeps them too and prints a warning. The default merge commit template has the reference. The default squash template is only the title, so with the fast-forward merge method the project has to add %{reference} there, README says it now. I checked seven merge setups in the runner image with sh and bash. Live GitLab is still to verify in the pilot. Fixes #52 * test(validate): cover CI template version check in scans checkCITemplateVersions had a provider lookup that can not fail for the two fixed templates, so its error return was never hit. Read the two template files directly. Also test the checkScans branch when the pin does not match the release manifest. * fix(ci): clean up GitLab download when the job fails The job runs under set -eu, so a failed checksum, version check, tar, or ci run exits before the rm and the archive stays in CI_BUILDS_DIR. An EXIT trap removes it on every path.



Summary
The v1.4.0 release run failed. Goreleaser published the release fine, then the new action major tag step could not push tag v1:
Reason: the step sends the token as
AUTHORIZATION: bearerin http.extraheader. GitHub git endpoints accept HTTP basic auth only, so the push gets 401 and git falls back to a credential prompt that cannot work on a runner. Bearer works on api.github.com, which is why goreleaser uploads with the same token succeeded.I checked this live against this repo: ls-remote with bearer fails, with
AUTHORIZATION: basic $(printf 'x-access-token:%s' "$TOKEN" | base64)works.Re-running the failed job then made it worse:
goreleaser release --cleanonly wipes local dist, so every asset upload hit 422 already_exists. Release v1.4.0 is published and complete, but tag v1 was never moved.Fix: send the token as x-access-token over HTTP basic, the same scheme actions/checkout uses, in the release tag step, the forge notes workflow (template plus committed copy), and the composite action. Also pin the scheme in the template security contract so it cannot slip back.
Tag v1 still points nowhere. After this merges I will push it at 155848f manually; re-running the release workflow would just hit the same 422 on assets.
Scope
Related issue
N/A
Validation
Checklist