chore(ci): correct CodeQL pinned-version annotations - #203
Conversation
Annotate the existing cdf488f595d80d6e07e03d4674febd5ab45fa938 pin as v4.37.9. The action SHA and executable workflow configuration are unchanged.
Review SummaryVerdict: LGTM ✅ VerificationI independently verified the SHA-to-version mapping:
Observations
Question for the authorThe PR is marked as draft — is there a specific reason, or can it be marked ready for review? The CI checks (V2 in your validation table) should be confirmed before merging. |
waterbro-8
left a comment
waterbro-8
left a comment
There was a problem hiding this comment.
APPROVE — independently re-derived, and it is a complete fix
The pin/tag mapping
Peeled the annotated tags in github/codeql-action:
| tag | tag object | peeled commit |
|---|---|---|
v4.37.9 |
a35ac6e6798d… |
cdf488f595d8… ← what both workflows pin |
v4.37.4 |
ea14db8afdef… |
f205ea1c3313… |
The pinned SHA is v4.37.9's commit, so the # v4.37.4 annotations have been wrong all
along and this PR corrects them. Nothing resolves to the v4.37.4 commit, i.e. nobody was
running an action version the label did not match — the executed code was always correct;
only the human-readable label lied.
No @<sha> is modified anywhere in the diff, so there is no behaviour change and no chance of
an unintended version bump riding along inside an annotation fix.
Completeness
I audited every uses: <owner>/<repo>@<40-hex> # <tag> across all 7 files in
.github/workflows/ on 3c13f04 — 52 sites — and re-derived each tag→commit mapping from
upstream. Exactly 4 sites are mislabelled, and they are exactly the 4 this PR fixes.
Nothing stale is left behind, so this closes the class rather than three-quarters of an
instance.
One adjacent nit, pre-existing and out of scope here, in case you want a follow-up issue:
ci.yml pins astral-sh/setup-uv@c771a70e62 # v9, but that repository publishes no v9 tag
at all — c771a70e62 is v9.0.0. The executed code is correctly pinned; only the major-line
claim in the comment is unverifiable upstream. Same failure mode in miniature as the four you
are fixing: a label that upstream does not actually have.
CI state
mergeable_state=blocked; Validate Agent memory fails on this head with
pull access denied for minio/minio, identical to #198/#199/#205. This PR edits comments in
two workflow files and cannot be implicated — flagging it only so the red is not read as
needing a code change.
Provenance
git ls-remote + github/codeql-action tag-object API on the four tags, plus a repo-wide
annotation audit on 3c13f04. I did not run any workflow, and I did not re-verify the 47
correct sites' file contents — only that each tag resolves to its pinned commit.
The one red check is external infrastructure, not this PRTracked in #207. What is failing, verbatim
Run Why this PR cannot be the cause
What this does not ask of youPlease do not rebase or re-push to chase this red, and do not add This PR is What I did not verify — please do not treat the above as complete
For context on urgency: the nine other open PRs (#189–#197) show this job green, but all of those Forensics: |
## What this changes
One line deleted from `.github/workflows/bytefolk-security.yml` — the
`if:` guard on the `codeql` job. Nothing else in the file or the
repository changes.
```diff
codeql:
name: CodeQL (${{ matrix.language }})
- if: ${{ github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository }}
runs-on: ubuntu-24.04
```
Blob size goes 1988 -> 1867 bytes. The deleted line is exactly 121 bytes
including its newline, so the arithmetic accounts for the whole delta:
no whitespace, reordering or annotation change rode along.
## Why
The guard's second clause compares the head repository to the base
repository. It is false for every fork pull request, so the `codeql` job
is skipped.
GitHub does not evaluate a job's `name:` expression when the job is
skipped by its `if:` guard. The skipped job publishes the raw template
string as its check name, so fork PRs in this repository report a check
literally named `CodeQL (${{ matrix.language }})`, which matches none of
the three required contexts `CodeQL (go)`, `CodeQL
(javascript-typescript)` and `CodeQL (python)`.
This is not hypothetical. Two fork PRs by `@sun-970` were measured
through `GET /repos/bytefolk/mem/commits/{sha}/check-runs` after their
first-time workflow runs were approved:
| PR | head | total | success | failed | skipped |
| --- | --- | --- | --- | --- | --- |
| #172 | `a29e9163` | 17 | 16 | 0 | 1 |
| #181 | `4a25bd25` | 17 | 16 | 0 | 1 |
The single skip in each is `CodeQL (${{ matrix.language }})`. Both are
otherwise fully green and both report `mergeable: true`, yet neither can
ever merge: the three CodeQL contexts are required and nothing the
contributor does produces them. The un-interpolated name is a symptom of
the skip, not a separate defect — on `bytefolk/digital-employee#250` the
same expression interpolated to `CodeQL (javascript-typescript)` as soon
as the job actually ran.
## Why deleting the guard rather than adding a `codeql-fork` job
`bytefolk/digital-employee#250` solved this by adding a second job with
the inverted guard. That job is identical to the baseline `codeql` job
apart from the guard and the language matrix, so a second lane
duplicates roughly 37 lines to express what deleting one line already
says.
The guard is also provably redundant for every trigger other than a fork
PR:
- `push`, `schedule`, `workflow_dispatch` — the first clause
`github.event_name != 'pull_request'` is already true.
- Same-repository `pull_request` — the second clause is already true.
- Fork `pull_request` — the job was skipped; it now runs. **This is the
only behavioural change.**
## Evidence that CodeQL actually works on a fork `pull_request` run
`bytefolk/digital-employee#250` is a fork PR (head
`PeterGuy326/digital-employee:feat/issue-245-memory-config`, sha
`abb2d58b29710927742c828a510c419c7c53efb8`). Its `codeql-fork` job
concluded `success` — check runs `102757987331` (84s) and `102757703884`
(86s).
That job declares `permissions: security-events: write` and contains
**no** `continue-on-error` on the job or on any step. A `success` job
conclusion therefore means every step succeeded, including `Analyze` —
and `github/codeql-action/analyze` fails with HTTP 403 when
`security-events: write` is absent. So SARIF upload from a fork
`pull_request` run is confirmed, not assumed.
Fork `pull_request` runs still receive no secrets and read-only
`contents`. All ByteFolk repositories are public.
## Tradeoff, stated plainly
After this change, fork PRs build untrusted code in a job holding
`security-events: write`. The run still has no secrets and read-only
`contents`, and every other required check in this repository (`Go`,
`Worker`, `Web`, `PostgreSQL integration`, `Web memory and transfer
acceptance`, `HTTP, CLI and MCP lifecycle`, `Workflow, scripts and
Compose`) already builds that same untrusted code, so this adds no new
class of exposure. It is the posture GitHub's own default CodeQL setup
takes for public repositories.
The alternatives were considered and rejected:
- `pull_request_target` would hand a write-scoped token to a workflow
run over attacker-influenced code.
- Self-reporting the three contexts through the Statuses API would
fabricate a required check that never ran.
- Granting contributors organization membership does not help at all:
the guard compares repositories, not author identity.
## What this does not touch
- No `permissions:` block changes. The job keeps exactly `contents:
read`, `actions: read`, `packages: read`, `security-events: write`.
- No required status context, branch protection rule or ruleset is
weakened or removed.
- No action SHA, trigger, matrix entry, step or job other than the
deleted line.
- The pinned `# v4.37.4` annotations are deliberately left alone. They
are the subject of bytefolk/.github#32 and #203, and this PR must stay
disjoint from them.
## Interaction with #203
#203 edits this same file, but only the three `github/codeql-action/*`
annotation lines at 58, 65 and 68, plus `bytefolk-scorecard.yml`. This
PR deletes line 36. The hunks do not overlap, so the two 3-way-merge
cleanly in either order. Neither needs rebasing because of the other.
## Checks
`Workflow, scripts and Compose` runs `actionlint` over every workflow.
Deleting a job-level conditional cannot introduce an actionlint finding;
the resulting YAML keeps `name`, `runs-on`, `timeout-minutes`,
`permissions`, `strategy` and `steps` on the `codeql` job. The release
pin validators in that job (`validate_release_action_pins.sh`,
`test_release_guards.sh`, `test_release_helpers_compat.sh`) all hardcode
`release.yml` and never read this file.
This branch is same-repository rather than a fork, so the PR's own three
CodeQL contexts can go green here — otherwise the fix could not
demonstrate itself.
## Follow-up
The identical guard sits in
`bytefolk/.github/workflow-templates/bytefolk-security.yml`, which is
where every consumer copied it from. A separate template PR is filed so
the fix propagates instead of regressing on the next template sync. Any
consumer that added its own `codeql-fork` lane (`digital-employee`, via
#250) must delete it once the template changes, otherwise two jobs
publish the same check name.
After this merges, #172 and #181 need a branch update rather than a
plain re-run: for `pull_request` events GitHub takes the workflow YAML
from the merge ref, while a re-run replays the workflow version captured
when the run was created. Both are already `state: behind` under
`strict: true`, so they need the update regardless.
Refs bytefolk/.github#35
Co-authored-by: 勒布朗-詹姆斯 <2986253039@qq.com>
3 participants
Canonical requirement
Refs bytefolk/.github#32
Decision reference: the initial R1 Issue body. It explicitly records that local candidates preceded this prospective publication record; no retrospective approval is claimed.
Requirement trace
f464f686(19 of 20 checks succeed, see Validation); independent human review requested and still pendingFile domains
.github/workflows/bytefolk-scorecard.yml(47);.github/workflows/bytefolk-security.yml(58, 65, 68).Prepared parent / merge base:
2986fe38175f54d99f15dd38a498708c6ecd88cdPR base at publication:
87db0dfe0507be2190fe2fdcce0e267be8224f4d. Since that baselinemainadvanced by six commits through3c13f04e(#162, #160, #165, #188, #158, #204) — not onlyweb/package-lock.jsonas previously stated here. None of them touched.github/workflows/, so the F9 workflow blobs and the PR diff are unchanged. The reviewed commit and original parent are preserved.Head:
f464f68636adc6bb5295c3818aa6654c46a3caad—d2a9ec5plus one non-forcedMerge branch 'main'commit (f464f686) that brought the branch up to3c13f04eso it is no longerBEHIND. Verified:git rev-parse d2a9ec5:.github/workflows/bytefolk-scorecard.ymland...:bytefolk-security.ymlreturn the same blobs (2058126c,48a507e0) as atf464f686, andgit diff main...f464f686is still exactly these 2 files,+4/-4. The comment-only payload is therefore byte-identical to the reviewed commit and the equality proof above holds on the current head.Scope and non-goals
Correct only
# v4.37.4to# v4.37.9on CodeQL uses-lines pinned tocdf488f595d80d6e07e03d4674febd5ab45fa938. The official tag object resolves to that existing pin. Action SHAs, permissions, triggers, steps, matrices, other pins, and runtime code are unchanged.Validation
ruby evidence/verify.rb --baselineandruby evidence/verify.rb --committedfrom the retained review packet;git diff --check 2986fe38175f54d99f15dd38a498708c6ecd88cd d2a9ec5adde7931280076c729209f448a31ca25efrom this repository.f464f686— 19 of 20 checks succeed. The single failure isHTTP, CLI and MCP lifecycle, whose log ispull access denied for minio/minioat ~13s: a container-image pull failure in an unrelated job. The same workflow was green onmainat3c13f04e, and the identical failure is present on fix(ci): run Web tests and preserve audit evidence (successor to #169) #198 and fix(release): validate compare links and protect checksum output (successor to #170) #199, so it is not caused by this comment-only change. Root-cause tracking is separate and open.The strict verifier checks the changed-file allowlist; exact old blobs and line inventory; complete expected-byte replacement; absence of stale target annotations; parsed YAML equality; comment-stripped byte equality and SHA-256 digests; whitespace and unchanged modes; one commit with the exact parent; and clean worktrees with no untracked files. All passed. The independent replay is recorded in canonical R1. The verifier and inventory are retained outside repository commits.
ruby evidence/verify.rb --committedf464f686HTTP, CLI and MCP lifecyclefails onpull access denied for minio/minio(infra, unrelated job, also failing on #198/#199, green onmain)Security and compatibility
Documentation annotation only. No dependencies, permissions, credentials, data flows, or runtime behavior change. The diff and commit identity were inspected for public-safe content. No CHANGELOG entry or behavior-documentation update is needed because only explanatory comments change.
Known limitations
Runtime suites, build, coverage, and dependency audits were not rerun for this comment-only change; no runtime test result is claimed. Hosted CI is separate from local equality proof. Two limits now apply: (1) the strict verifier's
one commit with the exact parentinvariant describes the reviewed payload commitd2a9ec5, not the current branch shape, which carries two additionalMerge branch 'main'commits; (2) this PR is not merge-ready yet — repositoryAGENTS.mdstep 6 requires passing CI and an approval from someone other than the author, andHTTP, CLI and MCP lifecycleis red on the unrelatedminiopull, so the green-CI half is unmet until that infrastructure failure is fixed.Risk and rollback
Low-risk annotation correction. Roll back through an ordinary revert of this single commit. There is no migration or release action.
Product review handoff
Maintenance update (2026-09-14, @waterbro-8)
Records written by the maintainer account, not by the implementation owner:
f464f686 Merge branch 'main'was pushed to this head branch (non-forced,mainat3c13f04eis an ancestor of the head) to clear theBEHINDstate this PR's own body said blocked merging. No workflow file content changed: both blobs are identical tod2a9ec5.web/package-lock.jsonin PR chore(web): refresh audited development dependencies #192" claim, theNOT VERIFIEDhosted-CI rows, and the "this is a draft, not merge-ready" note.AGENTS.mdstep 6 requires an approval from someone other than the author, and a commit author on the head cannot supply that approval for their own push.@PeterGuy326remains implementation and merge-ledger owner.