[FumingPower3925]

Refs goceleris/celeris#864. This corrects a security comment. The hole goceleris/celeris#864 describes needs a settings change (below, corrected in rounds 1 and 2), so this does not fix it.

Defect

.github/workflows/codspeed.yml at 1fe0430 says this about the macro-runner job (:50-51):

A macro runner is a self-hosted runner registered in this organization: code from a fork never runs on it.

That is not true.

• On pull_request, the workflow definition comes from the pull request's merge ref. A fork can delete the if: that skips forks, or add a workflow of its own that targets codspeed-macro-arm64-graviton-ubuntu-22-04.
• What holds a fork pull request is the approval policy (all_external_contributors), and that policy does not hold an organization member's fork pull request.
• WdnLiu is an organization member with read on loadgen (logs/settings-readonly.log). Their fork pull request can run on the macro runner with no approval.
• A maintainer who reads the comment would believe the if: is the boundary.

celeris's codspeed.yml was corrected for the same claim under celeris#725.

The header's "Until an organization owner allows public repositories in the Default runner group ... this job waits in the queue" (:17-19) is also stale. All three prerequisites were done on 2026-09-26 (celeris#690). Round 1 reads them from the API (fix-r1/scripts/settings-readonly-r1.sh, log fix-r1/logs/settings-readonly-r1.log, under the lane evidence dir lanes-20261003/CI1/):

• CodSpeedHQ/action@* is on this repository's allow-list (gh api repos/goceleris/loadgen/actions/permissions/selected-actions).
• CodSpeed run 36319097032 (a push on 2026-09-27) succeeded on a CodSpeed Runner with the label codspeed-macro-arm64-graviton-ubuntu-22-04.
• The Default runner group admits this repository. The run above shows that it did then. Reading the group itself needs admin:org, which this lane's token lacks (403).

Fix

Comments only. The header now says:

• what the if: does;
• why it is not the boundary;
• what is (the approval policy, and never approving a fork PR that changes .github/);
• what it does not cover (member fork pull requests, celeris#864);
• the three prerequisites, all marked done.

The if: is unchanged. It already skips every fork shape, a deleted fork included: head.repo.full_name == github.repository is false when head.repo is null.

Check

Truth table (lane evidence lanes-20261003/CI1/). It evaluates this file's real if: with act's GitHub-expression interpreter. Round 0: tests/exprtable, scripts/controls-864.sh, logs/864-summary.log. Round 1 re-ran it on today's mains and this PR's unchanged head, with the same rows for this file: fix-r1/tests/exprtable, fix-r1/scripts/controls-864-r1.sh, fix-r1/logs/864-r1-summary.log.

event	origin/main	this branch
push to main, workflow_dispatch, same-repo PR	runs	runs
same-repo draft PR	skipped	skipped
fork PR (external), fork PR (org member), fork PR (look-alike owner), deleted-fork PR	skipped	skipped

• Second control F (fix-r1/logs/864-r1-F-ctl-codspeed-unsafe-fork-check.log; round 0 logs/864-F-ctl3-codspeed-unsafe-fork-check.log) rewrites the guard as !github.event.pull_request.head.repo.fork. The deleted-fork row then runs, which shows the table can catch a guard that is not null-safe.
• actionlint is clean (go tool -modfile=.github/tools/go.mod actionlint).

Cost

None (comments).

Settings recommendation for the maintainer (goceleris/celeris#864), corrected in rounds 1 and 2

The full text, with its evidence, is on goceleris/celeris#864: issuecomment-5965624939. Round 2 added the second bullet of the standing rule, there and here.

The version first posted here, and on goceleris/celeris#864 as issuecomment-5965058060, got option a and rule c wrong.

• Write on probatorium gives its five repository secrets to any branch a writer pushes: CLUSTER_SSH_KEY, RUNNER_PAT, TS_OAUTH_CLIENT_ID, TS_OAUTH_SECRET and DOCS_DISPATCH_TOKEN.
• Nothing guards them. probatorium has no environments, and its rulesets cover only main.
• That is far more than the hole CI: the fork-PR approval policy exempts org members, so a read-only member's fork PR runs codspeed.yml on the macro runner without approval celeris#864 is about.

Recommended: make WdnLiu an outside collaborator.

gh api -X PUT orgs/goceleris/outside_collaborators/WdnLiu

• They keep write on celeris, through their former team contributors. Check it with gh api repos/goceleris/celeris/collaborators/WdnLiu/permission. If it did not carry over, grant it again with gh api -X PUT repos/goceleris/celeris/collaborators/WdnLiu -f permission=push.
• Their fork pull requests on loadgen, probatorium and docs then wait for "Approve and run".
• Side effect: they lose read on five private repositories that the org base permission gives them today: middlewares, benchmarks and examples (all three archived), SDD and TFM.

Not recommended: write on probatorium, unless the cluster secrets first move into an environment limited to main with required reviewers.

Standing rule:

• Organization membership is for people trusted with every repository's runners and secrets. Everyone else is an outside collaborator.
• (round 2) On a repository whose workflows reach a runner that is not GitHub-hosted, an outside collaborator gets write or no access, never read or triage. Those repositories are celeris and loadgen (the macro runner) and probatorium (the cluster); on probatorium write is ruled out too, so there it is no access.
  • GitHub's text for all_external_contributors exempts more than organization members: "All users that are not a member or owner of this repository and not a member of the organization will require approval to run workflows." Nothing read so far says whether a read collaborator counts as "a member of this repository".
  • Read is safe there only once a test shows that a read collaborator's fork pull request still waits for approval.
• Keep all_external_contributors on all four public repositories.
• Never approve a fork pull request that changes .github/.
• Keep the macro runners' Default group limited to celeris and loadgen.

Check after the change. The runner-group lines need a token with admin:org.

• gh api orgs/goceleris/members: only the maintainers.
• gh api orgs/goceleris/outside_collaborators: includes WdnLiu.
• gh api repos/goceleris/celeris/collaborators/WdnLiu/permission: write.
• gh api repos/goceleris/<repo>/actions/permissions/fork-pr-contributor-approval on celeris, loadgen, probatorium and docs: all_external_contributors.
• gh api orgs/goceleris/actions/runner-groups: Default has visibility selected.
• gh api orgs/goceleris/actions/runner-groups/1/repositories: celeris and loadgen only.

Optional, with no settings change: a pre-job hook (ACTIONS_RUNNER_HOOK_JOB_STARTED) on the cluster runners that refuses fork pull_request jobs.

Nothing was changed. The facts come from read-only calls: fix-r1/scripts/settings-readonly-r1.sh, log fix-r1/logs/settings-readonly-r1.log. The approval-policy wording comes from review-perf-api-security-r1/fresh/scripts/approval-docs.sh, log review-perf-api-security-r1/fresh/logs/approval-docs.log. Both paths are under the lane evidence dir lanes-20261003/CI1/.

Two expressions that look null-safe and are not.

• github.event.pull_request.head.repo.fork == false and !github.event.pull_request.head.repo.fork are both true for a pull request whose fork was deleted. head.repo is null, and GitHub's loose equality turns null into 0 and false into 0.
• The guards in use compare head.repo.full_name == github.repository, which is false for null. Keep that form.
• The truth table of fix(stress): the shard job's runner comes from the workflow, not from the pull request's plan (celeris#864) probatorium#486 shows both, and so do its second controls F and G.

Review (round 2)

Two reviews of 11c61f6 found nothing wrong in this PR's change. The head is still 11c61f6: this round changes only this text.

finding	what was done
MINOR: the standing rule's "grant everyone else access per repository, as an outside collaborator" could reopen the hole. GitHub's text also exempts "a member or owner of this repository", and it is not known whether a read collaborator counts.	Corrected in the settings section above and in the goceleris/celeris#864 comment: on a repository that reaches a non-GitHub-hosted runner, an outside collaborator gets write or no access.
NIT: the exact command and the checks were only on goceleris/celeris#864.	Copied into the settings section above.
NIT: the lane table's row pr-fork-same-name-owner-case tests a look-alike owner (goceleris-fork), not a case variant.	Renamed pr-fork-look-alike-owner in round 2's copy of the table. This body already called it that.
MINOR (probatorium): probatorium's own docs still said a fork PR never reaches the cluster, the claim this PR corrects for loadgen.	Fixed in goceleris/probatorium#486.

Truth table, round 2 (fix-r3/tests/exprtable, fix-r3/scripts/controls-864-r3.sh, summary fix-r3/logs/864-r3-summary.log, under the lane evidence dir). It ran on today's mains (celeris fa76fe6, probatorium 3f3a403, loadgen 1fe0430, docs 30f9bcc) and on this head. It now includes the review's adversarial rows for this file:

• The if: gives the same results as the table above.
• A head repo with no full_name, a null or empty full_name, a fork that reports fork: false, and a fork whose name extends this repository's: all skipped.
• This repository's name in other case: runs. Logins and repository names compare without case.
• Second control F rewrites the guard as !github.event.pull_request.head.repo.fork. Four of those rows then run, as does the deleted-fork row, so the table still catches a guard that is not null-safe.
• No test was skipped.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: goceleris/loadgen/.coderabbit.yaml
• Review profile: CHILL
• Plan: Advanced
• Run ID: 4e52c079-6d51-4cc6-8e32-aaf8a0a9ee6c

📥 Commits

Reviewing files that changed from the base of the PR and between 1fe0430 and 11c61f6.

📒 Files selected for processing (1)

• .github/workflows/codspeed.yml

🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

• goceleris/probatorium (manual) → reviewed against open PR #486 fix/probatorium-864-stress-runner-allowlist instead of the default branch
• goceleris/celeris (manual)

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review.

📜 Recent review details

⏰ Context from checks skipped due to timeout. (1)

• GitHub Check: Benchmarks (CodSpeed walltime, arm64 macro runner)

🧰 Additional context used

📓 Path-based instructions (1)

Every uses: is pinned to a full commit SHA with a version comment (the repository requires SHA pinning).

⚙️ CodeRabbit configuration file

Files:

• .github/workflows/codspeed.yml

🔇 Additional comments (1)

.github/workflows/codspeed.yml (1)

35-36: 🔒 Security & Privacy | 🛡️ Detected with Advanced Tier

The available evidence does not establish whether member fork workflows can run on the CodSpeed macro runner.

---

📝 Summary

Summary by CodeRabbit

• Documentation
  • Clarified the workflow’s runner-group and CodSpeed prerequisites, fork pull request approval policy and its limitations, and the warning about approving pull requests that modify .github/.
  • Explained that fork pull requests may be skipped by the workflow and that draft pull requests wait until they are ready. Workflow behavior remains unchanged.

Walkthrough

Comments in the CodSpeed workflow now describe runner and CodSpeed prerequisites, fork pull request approval guidance, the configured fork pull request skip, and draft behavior. The workflow logic did not change.

Changes

CodSpeed workflow guidance

Layer / File(s)	Summary
Runner and fork pull request guidance .github/workflows/codspeed.yml	Comments describe runner and CodSpeed prerequisites, fork pull request approval guidance and its limits, the configured fork pull request skip, and draft behavior.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~4 minutes

Change: Other

Suggested labels: dx

Merge Risk: ⚪ Minimal · up to 11c61

This change updates only workflow comments, so CodSpeed job behavior stays the same. The comments now explain correctly that the fork pull request skip is not a security boundary and name the settings change that is still needed. The change adds no merge risk; the underlying settings issue remains a separate follow-up.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 11c61

This PR changes documentation, not workflow execution or permissions. No introduced or worsened vulnerability was established. However, the claimed organization-member fork execution path depends on approval and runner-access settings that could not be independently verified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Findings and Attack Paths

• observed — The member-fork execution candidate remains deferred, not a verified finding. Establishing the proposed path requires effective approval settings, membership and repository permission evidence, and runner-group admission evidence. The supplied evidence does not establish broader runner, credential or cross-repository exposure.

Trust Boundaries and Controls

• observed — As written, the pull-request job requires a same-repository head and excludes drafts. Workflow permissions default to empty; the job grants contents read and OIDC token issuance, and checkout disables persisted credentials. These unchanged controls describe this job, but do not independently establish protection against a fork-modified workflow targeting the runner.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title uses the required conventional-commit format, ci(codspeed):, and its imperative summary accurately describes the comment updates about fork pull requests and the macro runner.
Description check	✅ Passed	The description explains the security-comment correction, the unchanged workflow guard, and the stale runner-prerequisite note. It is directly related to the changeset.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Readme Cluster-Bench Contract	✅ Passed	The reviewed diff changes only .github/workflows/codspeed.yml comments. It does not change loadgen.Result, nested result types, CLI flags, or flag defaults, so the README contract update condition…

✨ Finishing Touches 💡 1

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

🛠️ Fix failing CI checks 💡

• Commit to this branch
• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

_{Comment @coderabbitai help to get the list of available commands.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[codspeed]

Merging this PR will degrade performance by 82.27%

❌ 2 regressed benchmarks
✅ 17 untouched benchmarks

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

	Benchmark	BASE	HEAD	Efficiency
❌	1024workers	948 ns	20,237 ns	-95.32%
❌	BenchmarkH2CUpgradeRequest	36.7 µs	54.7 µs	-32.86%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.

---

_{Comparing fix/loadgen-codspeed-fork-comment-celeris-864 (11c61f6) with main (9f8993e)¹}

Footnotes

1. No successful run was found on main (1fe0430) during the generation of this report, so 9f8993e was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

[FumingPower3925]

The CodSpeed "Performance Analysis" failure here is runner noise, not this change.

• Same code: base 9f8993e and this head have identical Go code. main's only commit since then, 1fe0430 (ci: pin actionlint in a tools module and run it with go tool (#103) #104), touches no Go source, and this PR edits comments only.
• Tracking: the two flagged benchmarks are tracked in CodSpeed: ShardedLatencyRecorder/1024workers and H2CUpgradeRequest swing 21x and 1.5x on identical code (walltime on the macro runner), so every report can flag them #109.
• Merge: the required checks (Lint, Test, and Integration with live celeris) are green, so this merges without it.