Skip to content

Use x-access-token to fix some git fetch errors when prompt is disabled. - #171

Merged
ggreer merged 2 commits into
mainfrom
ggreer/github-access-token
Oct 22, 2026
Merged

ggreer merged 2 commits into
mainfrom
ggreer/github-access-token

Conversation

@ggreer

@ggreer ggreer commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

Example failure: https://github.com/ConductorOne/baton-fivetran/actions/runs/39768742068/job/119790645257

Also disable modules-download-mode: readonly in the golangci yaml. It was causing us to re-fetch vendored dependencies, which when combined with the auth error, failed lint in some cases.

@ggreer
ggreer requested a review from a team October 22, 2026 21:59
@github-actions

github-actions Bot commented Oct 22, 2026 •

Copy link
Copy Markdown
Superseded — see the current review report for commit a0b79411e413

General PR Review: Use x-access-token to fix some git fetch errors when prompt is disabled.

Blocking Issues: 0 | Suggestions: 0 | Threads Resolved: 0
Criteria: Criteria status: none loaded - .claude/skills/ci-review.md was not found at trusted base ad85b5b02764.
Review mode: full
View review run

Review Summary

This PR changes the git insteadOf URL rewrite in both the lint and test jobs of .github/workflows/verify.yaml. The URL changes from https://${TOKEN}@github.com/ to https://x-access-token:${TOKEN}@github.com/, so the token goes in the password field instead of the username field. With the token as the username, git has no password, so it tries to prompt for one and fails when prompting is disabled (GIT_TERMINAL_PROMPT=0, which Go module fetches use). Putting the token in the password field fixes that, and GitHub accepts x-access-token as the username for both PATs and GitHub App installation tokens.

I scanned the full diff for security and correctness issues. Both call sites are updated the same way, and no other insteadOf or @github.com rewrites remain anywhere in the repo. The TOKEN value still comes from the secret through an env variable rather than being interpolated inline, so the injection exposure is unchanged. No repo-local criteria were loaded, so this review used the base criteria only. There were no prior findings, and I found no new issues.

Security Issues

None found.

Correctness Issues

None found.

Suggestions

None.

@github-actions

github-actions Bot commented Oct 22, 2026 •

Copy link
Copy Markdown
Superseded — see the current review report for commit 45a6e9ae0742

General PR Review: Use x-access-token to fix some git fetch errors when prompt is disabled.

Blocking Issues: 0 | Suggestions: 0 | Threads Resolved: 0
Criteria: Criteria status: none loaded - .claude/skills/ci-review.md was not found at trusted base ad85b5b02764.
Review mode: full
View review run

Review Summary

This PR changes the git insteadOf URL rewrite in both the lint and test jobs of .github/workflows/verify.yaml. The URL changes from https://${TOKEN}@github.com/ to https://x-access-token:${TOKEN}@github.com/, so the token goes in the password field instead of the username field. With the token as the username, git has no password, so it tries to prompt for one and fails when prompting is disabled (GIT_TERMINAL_PROMPT=0, which Go module fetches use). Putting the token in the password field fixes that, and GitHub accepts x-access-token as the username for both PATs and GitHub App installation tokens.

I scanned the full diff for security and correctness issues. Both call sites are updated the same way, and no other insteadOf or @github.com rewrites remain anywhere in the repo. The TOKEN value still comes from the secret through an env variable rather than being interpolated inline, so the injection exposure is unchanged. No repo-local criteria were loaded, so this review used the base criteria only. There were no prior findings, and I found no new issues.

Security Issues

None found.

Correctness Issues

None found.

Suggestions

None.


Reviewed commit: a0b79411e413

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blocking issues found — see the full review report

@github-actions

github-actions Bot commented Oct 22, 2026 •

Copy link
Copy Markdown
Superseded — see the current review report for commit 45a6e9ae0742

General PR Review: Use x-access-token to fix some git fetch errors when prompt is disabled.

Blocking Issues: 0 | Suggestions: 0 | Threads Resolved: 0
Criteria: Criteria status: none loaded - .claude/skills/ci-review.md was not found at trusted base ad85b5b02764.
Review mode: incremental since a0b7941
View review run

Review Summary

The PR changes both insteadOf rewrites in .github/workflows/verify.yaml (lint and test jobs) to use https://x-access-token:${TOKEN}@github.com/. Before, the token went in as a bare username with no password. The new form is the documented way to authenticate GitHub App and PAT tokens over HTTPS, so git no longer stops to ask for a password when GIT_TERMINAL_PROMPT=0. The new commit since the last review (45a6e9a) removes run.modules-download-mode: readonly from .golangci.yml. With that setting removed, a repo that has a vendor/ directory gets Go's default -mod=vendor instead of fetching the dependencies again. That change is outside the PR title's scope. It is also harmless to callers: the lint job runs golangci-lint with working-directory: _caller, so it reads the caller repo's own config, and no workflow in this repo references this file. I scanned the full PR diff for security and correctness. The token is still read from the RELENG_GITHUB_TOKEN secret through env and is never echoed, so its exposure is unchanged. No repo-local criteria loaded, so only the base criteria were applied.

Security Issues

None found.

Correctness Issues

None found.

Suggestions

None.

@github-actions

github-actions Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

General PR Review: Use x-access-token to fix some git fetch errors when prompt is disabled.

Blocking Issues: 0 | Suggestions: 0 | Threads Resolved: 0
Criteria: Criteria status: none loaded - .claude/skills/ci-review.md was not found at trusted base ad85b5b02764.
Review mode: incremental since a0b7941
View review run

Review Summary

The PR changes both insteadOf rewrites in .github/workflows/verify.yaml (lint and test jobs) to use https://x-access-token:${TOKEN}@github.com/. Before, the token went in as a bare username with no password. The new form is the documented way to authenticate GitHub App and PAT tokens over HTTPS, so git no longer stops to ask for a password when GIT_TERMINAL_PROMPT=0. The new commit since the last review (45a6e9a) removes run.modules-download-mode: readonly from .golangci.yml. With that setting removed, a repo that has a vendor/ directory gets Go's default -mod=vendor instead of fetching the dependencies again. That change is outside the PR title's scope. It is also harmless to callers: the lint job runs golangci-lint with working-directory: _caller, so it reads the caller repo's own config, and no workflow in this repo references this file. I scanned the full PR diff for security and correctness. The token is still read from the RELENG_GITHUB_TOKEN secret through env and is never echoed, so its exposure is unchanged. No repo-local criteria loaded, so only the base criteria were applied.

Security Issues

None found.

Correctness Issues

None found.

Suggestions

None.


Reviewed commit: 45a6e9ae0742

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blocking issues found — see the full review report

@ggreer
ggreer merged commit 49d2c72 into main Oct 22, 2026
7 checks passed
@ggreer
ggreer deleted the ggreer/github-access-token branch October 22, 2026 22:09
gontzess added a commit that referenced this pull request Oct 22, 2026
## Incident
Incident triage reported 13 failed release runs across two classes:
vendored private-module resolution failures and transient toolchain
download/stream errors.

An explicit `GOFLAGS=-mod=readonly` disables Go's automatic vendor mode.
A caller with a complete vendor tree therefore attempts module
resolution instead of using its vendored private dependencies. The job's
proxy-only `GOPROXY=https://proxy.golang.org` also removes Go's normal
direct fallback, exposing private-module 404s. Vendor mode and readonly
mode both refuse required go.mod edits; forcing a module mode job-wide
is not needed for that hygiene property.

## Fix
- Remove only the three job-level `GOFLAGS`, `GOPROXY`, and `GOSUMDB`
assignments from `goreleaser-binaries`. The required whole-file
inventory found no matching assignments in any other release job.
- Keep `GOTOOLCHAIN=local`, locked tool versions/checksums, module
gates, and the trusted gon build's separate step-local module/checksum
settings unchanged.
- Retry artifact downloads with `curl --retry 5 --retry-delay 2
--retry-all-errors`; PowerShell uses `Invoke-WebRequest
-MaximumRetryCount 5 -RetryIntervalSec 2`. Hash-before-extract and the
committed download origins remain unchanged.
- Add four lines to the existing CI-owned no-self-reference/workflow
test to reject workflow/job-level module-resolution overrides. No new CI
job or checker framework is introduced.

## Related fixes and merge order
[#171](#171) owns
the separate Git credential-form fix and the root `.golangci.yml`
readonly removal. Neither is duplicated here.

That root configuration is upstream of the fleet sync and pairs with
[the baton-admin template
guard](ConductorOne/baton-admin#1020): **#171
merges first, then the downstream template PR**. This incident fix has
no downstream-template dependency. This repository's module graph
contains only public dependencies and no tracked vendor tree.

This branch was created from main and rebased onto
`49d2c72e6150c82c8cae8e37fdb0dc5ce42aab79` after #171 merged; its
incident patch is unchanged in range-diff. It is not stacked on an open
feature branch. The incident change is limited to caller module-mode
defaults, bounded installer retries, and their existing CI/fixture
coverage.

## Evidence
- `make test-scripts`: PASS, including 28 tool-integrity tests (native
Windows PowerShell fixture skipped locally).
- `GOTOOLCHAIN=local go test ./...` and `GOTOOLCHAIN=local go vet
./...`: PASS.
- `make workflow-validate`, Bash syntax check of `install-tool.sh`, and
`git diff --check`: PASS.
- Actual CI assertion smoke: 33 workflow/job environment mutations were
rejected; normal and trusted step-local settings passed.
- Real Go fixture smoke with network disabled: a synthetic private
vendored dependency builds under default vendor mode; adding
`-mod=readonly` bypasses vendoring and fails module resolution. go.mod
remains unchanged.
- Real curl fixture smoke: an empty-response stream failure is retried
successfully with the new flags; removing `--retry-all-errors` fails
immediately with exit 52 before extraction/PATH publication. Successful
bytes still pass the fixture's committed checksum.
- The native Windows harness now checks both retry parameters. Native
Windows/PowerShell execution is unavailable locally; the existing
Windows CI job exercises that harness.

### Known full-lint limitation (pre-existing)
Full `golangci-lint run --timeout=5m ./...` with v2.14.0 exits 3 because
gocritic/ruleguard references the absent `tools/rules.go`.

The comparison using main `49d2c72e`'s configuration also exits 3 with
the same `ruleguard init error: no file matching .../tools/rules.go`. Go
sources are unchanged from main. This is not introduced by the
module-mode fix; no unrelated lint rules are changed here.

## Scope and follow-ups
- No second-source/mirror code, lock-file column, dist mirror, or
hosted-toolcache fallback is added. A second source is banked only if
stream errors persist after these retries.
- No outer GoReleaser retry wrapper: it was considered and rejected
because partially published releases can collide with
immutable/versioned outputs.
- A vendored-private-module canary belongs in a separate
baton-github-test PR and a separately authorized live release sequence;
it is not created or dispatched here.
- No v4 tag change, merge, deployment, credential configuration, or live
release is performed by this draft PR. Prepared fixtures and local smoke
tests are not deployed-canary evidence.

Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants