ci(security): run CodeQL on fork pull requests

[PeterGuy326]

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.

   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 chore(ci): correct CodeQL pinned-version annotations .github#32 and chore(ci): correct CodeQL pinned-version annotations #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

[sun-970]

Reviewed the change and the analysis. LGTM.

The if: guard was causing fork PRs to skip CodeQL entirely, and the un-interpolated ${{ matrix.language }} in the job name produced a check that never matched the three required contexts — effectively blocking all fork PRs from merging with no way for contributors to resolve it.

Security posture is fine: fork PRs still get no secrets and read-only contents: read. The security-events: write permission is required by github/codeql-action/analyze for SARIF upload and does not introduce a new attack surface — every other required check in this repo already builds the same untrusted fork code. This is also the default posture GitHub recommends for CodeQL on public repositories.

One minor note: it might be worth adding a brief inline comment on the codeql job (or in the commit message) explaining why the guard was removed, so future readers don't re-add it thinking it was accidentally deleted. Something like:

# No `if:` guard — CodeQL must run on fork PRs to satisfy required status contexts.

But that's a nit, not a blocker. Merging as-is is fine.

[Bindy-lbb]

LGTM. 与 .github#36 同系列改动：

• 移除 bytefolk-security.yml 中 CodeQL job 的 fork PR 过滤条件。
• 使 fork 来的 PR 也能运行 CodeQL 扫描，安全覆盖范围扩大。
• CodeQL 使用 read-only token，无 secret 暴露风险，符合 GitHub 官方建议。✅

[waterbro-8]

APPROVE — the guard is a coverage bypass; one caveat about what this PR's own CI can prove

Why the if: has to go

On the fork PR #181 (head 4a25bd25), reported against bytefolk/mem: 17 check-runs, every
one of them completed/success, and CodeQL appears exactly once — as the unexpanded
CodeQL (${{ matrix.language }}) with completed/skipped. None of CodeQL (go),
CodeQL (python) or CodeQL (javascript-typescript) ever report.

So as configured, opening a PR from a fork yields a fully green PR with zero CodeQL coverage.
That is an attenuation path anyone can take without intending anything sneaky, and deleting
the condition is the right fix.

Why deleting it is safe here

• The job already declares contents: read, actions: read, packages: read,
  security-events: write — the permission set a pull_request event needs to upload SARIF.
• fail-fast: false, so one language failing does not hide the other two.
• bytefolk/mem is visibility=public, so code scanning on fork PRs does not depend on an
  Advanced Security licence.
• persist-credentials: false on checkout is untouched.

The caveat, stated plainly

#205 is a same-repo branch PR (head.repo.full_name == bytefolk/mem), so its own run —
where CodeQL does expand into CodeQL (go) / CodeQL (python) /
CodeQL (javascript-typescript) plus the aggregate, all success — exercises the path that
already worked before this change. It is not evidence about the fork path.

Could you watch the first fork PR after merge and confirm the three matrix contexts report?
If the SARIF upload is rejected in a fork context, the fix is the upload/permission setup, not
restoring this if: — restoring it reopens the bypass.

Two things that are not your fault but do touch this PR

1. mergeable_state=blocked, and Validate Agent memory / HTTP, CLI and MCP lifecycle is
   completed/failure on this head. Log:
   minio Error pull access denied for minio/minio, repository does not exist or may require 'docker login': denied: requested access to the resource is denied — Docker Hub anonymous
   pull refused at container start, before any project code runs. A one-line if: removal in a
   job that never touches MinIO cannot be implicated. Same failure on #198, #199 and #203.
2. #203 rewrites the four # v4.37.4 annotations in this same file (init/autobuild/
   analyze) and one in bytefolk-scorecard.yml, a few lines from where this guard is
   deleted. Different lines, so I expect no text conflict, but whichever merges second should
   get a fresh CI run rather than an admin override.

Provenance

Static read of head 7e9ce11e plus check-run / workflow-run / repo-metadata API calls. I did
not execute any workflow, and I could not read org-level rulesets (admin:org), so I cannot
confirm from config whether the three CodeQL contexts are required — I only confirmed that on
a fork PR they never appear at all.

[waterbro-8]

The one red check is external infrastructure, not this PR

Tracked in #207.

What is failing, verbatim

Validate Agent memory → job HTTP, CLI and MCP lifecycle (job id acceptance) cannot start its
test stack — docker compose is refused an anonymous pull of minio/minio; postgres shows
Interrupted only as a consequence:

 minio Error pull access denied for minio/minio, repository does not exist or may require 'docker login': denied: requested access to the resource is denied

Run 34857342711 / job 104034397117, failed 2026-09-14T15:20:06Z after 15 s — the newest
occurrence of this failure in the repository, i.e. the outage was still live at that point.

Why this PR cannot be the cause

• Head 7e9ce11e has 20 check-runs: 19 success, 1 failure — the only failure is this job.
  Notably CodeQL, CodeQL (go), CodeQL (javascript-typescript) and CodeQL (python) are all
  green, which is the machinery this PR actually changes.

• This PR is 0 additions / 1 deletion in 1 file (.github/workflows/bytefolk-security.yml): it
  removes a fork guard.

  -    if: ${{ github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository }}

  It does not touch memory-validation.yml, docker-compose.test.yml,
  scripts/acceptance_agent_memory.sh, or any image reference. There is no path from this hunk to
  a container pull.

• The same head SHA is success on the CI workflow (321876000) — byte-for-byte the commit this
  job fails on. Only the minio-dependent job is red.

What this does not ask of you

Please do not rebase or re-push to chase this red, and do not make the failing job tolerant
(continue-on-error, || true, a skip) to land this change — the gate and the fix belong in #207.

This PR is APPROVED with mergeStateStatus: BLOCKED and this as its only red check. I am not
claiming the block is caused by that check; see below.

Worth separating, so the red does not bury what #205 actually buys. I checked this rather than
assuming it: fork PRs are not CI-starved today — cross-repo #181 has green check-runs, including
this very job. The gap is narrower and is exactly this PR's subject. On a fork head
(4a25bd25) the CodeQL machinery produces a single entry, CodeQL (${{ matrix.language }}), with
conclusion skipped — the unexpanded matrix expression is visible in the name — and the three
expanded contexts (CodeQL (go), CodeQL (javascript-typescript), CodeQL (python)) never appear
at all. Removing this guard is what makes security analysis run on contributor-proposed changes
instead of only on in-org branches. If you merge it, please watch the first fork PR afterwards for
SARIF-upload rejection, which was the original reason the guard existed.

What I did not verify — please do not treat the above as complete

• Whether it is still red now. Newest Validate Agent memory run anywhere is 2026-09-14T15:17Z;
  nothing has re-run since ci(infra): minio pull denied blocks HTTP, CLI and MCP lifecycle acceptance #207 was filed, so recovery status is unknown to me. I did not re-run
  anyone's workflow to find out.
• Which side is at fault. pgvector/pgvector pulls fine from the same runners in the same
  window, ruling out general egress and a rate-limit toomanyrequests, but Docker Hub returns one
  string for "removed", "restricted" and "needs login" and I could not query the registry from here.
• Whether minio/mc (docker-compose.test.yml:43) is denied too — unobservable; the run dies
  on the minio service before minio-init.
• Whether this job is required-config enforced — branches/main/protection is 404 to me
  (GOVERNANCE.md:41-42 predicts this for non-admins) and org rulesets need admin:org.

Nine other open PRs (#189–#197) show this job green, but those runs are all dated 2026-09-10 —
stale, not healthy; most are drafts that will hit the same wall when marked ready.

Forensics: the failing run's log, check-runs on head 7e9ce11e, this PR's diff, and git log on
the three failure-path files. Nothing run locally, no CI re-run.