Commit results to fork PRs with an optional fork-push token#1011
Merged
Conversation
The collector could only push result files to PR branches in this repository: its GITHUB_TOKEN is scoped to ClickHouse/ClickBench, and GitHub grants the "allow edits from maintainers" push permission only to user accounts with write access, never to App installation tokens. Fork PRs got a pastila.nl link and a request to commit the file by hand. With a classic PAT of such a user in the CLICKBENCH_FORK_PUSH_TOKEN secret, the collector now commits results to fork PRs too, when the author allows maintainer edits. The push clears the http extraheader that actions/checkout stores in the git config (it carries GITHUB_TOKEN and would be sent to the fork and rejected) and supplies the token through an inline credential helper reading the environment variable, so it never appears in a command line or an error message. If the push fails — the author unticked the box meanwhile, the branch moved, the fork protects it — the old paste-link fallback is posted instead of failing the PR. When the token is configured but the author disallows maintainer edits, the comment asks them to tick the box. Without the secret, behavior is unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
`ROBOT_CLICKHOUSE_COMMIT_TOKEN` is an org secret already shared with this repository, so no new secret or machine account is needed. The `robot-clickhouse` account currently has read-only access here and must be granted write access for the fork pushes to be accepted; until then the collector falls back to paste links as before. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e reverted)" This reverts commit 4532951.
This reverts commit f3a5a00.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
collect-results.ymlcould only commit result files to PR branches in this repository: itsGITHUB_TOKENis scoped to ClickHouse/ClickBench, and GitHub grants the "allow edits from maintainers" push permission only to user accounts with write access — never to App installation tokens. Fork PRs (e.g. update: Sail to 0.6.6 #982) got a pastila.nl link and a request to save the file by hand.maintainer_can_modify), authenticating with the org-levelROBOT_CLICKHOUSE_COMMIT_TOKENsecret, which is already shared with this repository — no new secret or machine account needed, and no new exposure (the secret was already readable by this repo's workflows).http.https://github.com/.extraheaderthatactions/checkoutstores in the git config — it carries theGITHUB_TOKENand would be sent to the fork and rejected — and supplies the PAT through an inline credential helper that reads the environment variable, so the token never appears in a command line or an error message.Setup after merge
robot-clickhousecurrently has read-only access to this repository, and the maintainer-edit fork push requires write access to the base repo. Grant it:Until then, the behavior is unchanged (fork pushes fail gracefully to paste links).
Test plan
git credential fillconfirms the inline helper hands the token from the environment to gitfetch_branchfetches PR update: Sail to 0.6.6 #982's head straight from the fork's public URLcommit_results(DRY_RUN) builds the result commit on top of the fork head and targets the fork remotefork_pushis true with the token set, false without itROBOT_CLICKHOUSE_COMMIT_TOKENconfirmed visible to this repository (org secret list)d6de6f2d4e—robot-clickhousenow has write access, and the maintainer-edit push works🤖 Generated with Claude Code