field notes · falcosecurity/plugins · issue #1505

The Anatomy of a Guard That Never Passed

Manny Castillo · Senior SDET · September 2026

 

confirmed by the maintainer, two PRs open, byte-identical to the libs fix

updated4 September 2026. A second, unrelated drift turned up in the same workflow file days after publishing, which is the post's own argument arriving as evidence. New postscript in section 7.

ShareLinkedIncopy link

A security fix hardened a workflow against a real attack, and in doing so rejected every outside contributor for six months. Nobody noticed, because the only people who could see it working were the people it was never checking. This one starts with a CI failure on my own pull request that I nearly ignored. Budget ~14 minutes.

1. A CI failure worth reading

Two failure emails landed for one pull request. The first was a set of container tests failing on arm64 and amd64, which were also failing on other people’s pull requests and were plainly not mine. The second was smaller and stranger:

Artifact PR number 1501 does not match any PR for commit
d04fb7840b5b322af9012de15cb7d241b270111d.
Aborting to prevent artifact poisoning.

That is not a test failing. That is a workflow refusing to run because it believes something is being spoofed. Worth thirty seconds before dismissing it.

Thirty seconds was all it took to establish the message was wrong. d04fb784 was the head of my pull request. It had not been force pushed, nothing had been rebased, and the commit was reachable in the repository. Every claim the error made about my branch was false.

When an error is confidently wrong about something you can verify in one command, the interesting question stops being “what did I do” and becomes “what is this code actually looking at?”

2. What the workflow is defending

Before the bug, the threat, because the guard exists for an excellent reason and the fix has to preserve it.

The workflow posts a bot comment on a pull request. Both the comment body and the target pull request number arrive as an artifact, a file uploaded by an earlier workflow. That earlier workflow builds the contributor’s code. On a fork pull request, that means it runs code written by someone who is, from the repository’s point of view, a stranger.

So the artifact is attacker controlled. It contains a file called NR holding a number, and the workflow comments on whatever number it finds there.

var issue_number = Number(fs.readFileSync('./NR'));
// ... later
await github.rest.issues.createComment({
  owner: context.repo.owner,
  repo: context.repo.repo,
  issue_number: issue_number,
  body: comment_body.toString('utf8')
});

Write 42 into that file and the repository’s own bot posts your text on issue 42. Not your pull request. Any thread in the repository, in the voice of github-actions[bot], which people trust more than a stranger.

That is comment injection through artifact poisoning, and PR #1226 closed it in March with a validation step. The idea is exactly right: before commenting, confirm that the number in the artifact really is the pull request this run belongs to.

3. The wrong repository

Here is the check as it shipped:

const head_sha = context.payload.workflow_run.head_sha;
const {data: associated_prs} =
  await github.rest.repos.listPullRequestsAssociatedWithCommit({
    owner: context.repo.owner,   // falcosecurity
    repo: context.repo.repo,     // plugins
    commit_sha: head_sha,
  });
const valid_pr_numbers = associated_prs.map(pr => pr.number);
if (!valid_pr_numbers.includes(issue_number)) {
  core.setFailed(`Artifact PR number ${issue_number} does not match ...`);
  return;
}

In English: take the commit this run was for, ask falcosecurity/plugins which of its pull requests contain that commit, and require the artifact’s number to be one of them.

For a pull request from a fork, that commit lives in the contributor’s repository. Asking the base repository about it returns an empty list. And a number is never a member of an empty list.

Same commit, same endpoint, two repositories, opposite answers. The guard could never pass for a fork pull request. Not sometimes. Not under load. Never, by construction, since 2 March 2026.

Why six months of silence. Maintainers push branches directly to the repository, so context.repo is the right repository for them and the check works perfectly. The only people who could observe the failure were outside contributors, who tend to assume a red mark on someone else’s CI is their own fault and say nothing. The bug was invisible to everyone with the power to fix it and unmentionable by everyone who could see it.

4. Trace it yourself

Choose where the pull request comes from, whether the artifact is honest or poisoned, and which version of the guard is running. The panel shows which lookup executes, what it returns, and whether the comment gets posted.

The row worth finding is the last one: a poisoned artifact against the fixed guard. A fix that unbreaks contributors by letting attacks through is not a fix.

Four things fall out of playing with it. The old guard is correct exactly once, on a same-repo pull request, and that is the only case its authors could ever see. It blocks the attack, but it also blocks everything else. The new guard blocks the attack and passes honest fork contributions. And the reason it can do both is not that it checks more carefully. It is that it asks a different question.

5. The fix I proposed, and why it was refused

My report suggested the obvious repair. The lookup fails because it asks the wrong repository, so ask the right one:

owner: context.payload.workflow_run.head_repository.owner.login,
repo: context.payload.workflow_run.head_repository.name,

Query the fork instead of the base. It works. I checked it against three real pull requests before writing the issue.

The maintainer confirmed the diagnosis, then declined the fix, and his reason is the most useful thing I took from the whole exercise:

He is right, and I had half seen it myself. My own issue text proposed those extra checks as belt and braces. What I had not noticed is what needing them means.

If a lookup can return an answer you have to filter, the lookup is wrong. Every filter is a rule someone can forget, misread, or delete during a refactor two years from now, and if they do, the check silently stops checking. The version that needs no filter cannot decay that way.

The version they already had

They had also solved this before. falcosecurity/libs hit the identical bug a week after #1226 and fixed it there. Nobody propagated it to plugins or rules. The fix never leaves the base repository:

const run_prs = context.payload.workflow_run.pull_requests;
let valid_pr_numbers = run_prs.map(pr => pr.number);

if (valid_pr_numbers.length === 0) {
  // Fork PR: search using the head repo owner and branch from the
  // workflow_run payload (these fields are set by GitHub, not the fork).
  const head_owner = context.payload.workflow_run.head_repository.owner.login;
  const head_branch = context.payload.workflow_run.head_branch;
  const {data: matching_prs} = await github.rest.pulls.list({
    owner: context.repo.owner,
    repo: context.repo.repo,
    state: 'all',
    head: `${head_owner}:${head_branch}`,
    per_page: 10,
  });
  valid_pr_numbers = matching_prs.map(pr => pr.number);
}

Two paths. Same-repo pull requests get their numbers from workflow_run.pull_requests, which GitHub populates and which needs no API call at all. Fork pull requests, where that array is always empty, fall through to a different question asked of the base repository: list your own pull requests whose source branch is ManuelFCastillo:fix/container-linux-resolv.

The base repository knows about its own pull requests no matter where the branch physically lives. That is the insight my version missed.

Two properties, not one

It is worth separating what makes this safe, because the two halves fail differently.

The inputs cannot be forged. head_repository.owner.login and head_branch come from the workflow_run payload, which GitHub fills in. The fork’s code never touches them. Compare that with the artifact, which the fork writes freely, and the distinction the whole workflow turns on becomes visible: some things in a CI run are stated by the platform and some are stated by the code being tested, and only one of those is evidence.

The outputs are scoped by construction. pulls.list is called on the base repository, so every number it can possibly return is a pull request of that repository. There is no filtering step because there is nothing to filter.

Mine was safe after checking. Theirs is safe before checking. Those are not the same kind of safe, and the difference is entirely in which question gets asked.

6. Shipping it

Three artifacts came out of a CI failure I nearly deleted: plugins#1505 for the report, plugins#1509 and rules#383 for the fix, porting the libs logic byte for byte so the three repositories stop drifting.

There is a pleasing loop in it. The workflow that #1509 repairs is the same one that was failing on my other open pull request. Merging the fix makes my own contribution stop erroring, which is a strange and satisfying way to unblock yourself.

The diff is 23 lines added and 8 removed, in one file, twice. Most of the work was reading.

7. What generalises

Read the CI failure that is not about your code. The container test failures on that pull request genuinely were not mine and genuinely were not interesting. The other one was eleven words about artifact poisoning, and it was a six month old regression affecting every outside contributor. The cost of checking was one API call.

An error confidently wrong about a verifiable fact is a gift. It told me the commit did not belong to the pull request. One command proved otherwise. At that point the bug is not in your branch, it is in the thing doing the looking, and you have already narrowed it to a single question: what is that code actually reading?

Hardening changes need to be tested from outside the walls. This one was correct for everybody who could run it and broken for everybody who could not. Whenever a change touches permissions, provenance, or trust boundaries, the group that can no longer do something is precisely the group least able to report it.

If you have to filter the answer, you asked the wrong question. The maintainer’s objection generalises well past GitHub Actions. A lookup whose results must be post-filtered for safety is one refactor away from being unsafe. A lookup that cannot return the wrong thing stays correct without anyone maintaining it.

Check whether the project already solved it. The fix existed in a sibling repository for six months. Proposing something novel when a proven version is sitting one repository over creates review work for no gain. Ask before inventing.

A postscript that proved the point

Question 4 below asks what it means that three repositories carry byte-identical workflow logic, and answers: it was copied, not shared, so a fix in one is invisible to the others. That was a claim about shape when I wrote it. Days later it produced a second example on its own.

Watching the pull request land, I noticed the same file fails a different way. The Download artifact step filters run artifacts for one named pr and takes [0] without checking anything came back:

Twelve of the last twenty runs of that workflow had failed on it, all on main. libs guards it, with an early exit when the filter returns nothing. Plugins never got that either.

So the file had drifted in at least three places, and fixing the one I came for did not fix the others, because nothing connects them. That is the difference between a bug and a shape: a bug you fix once. A shape keeps producing bugs until someone changes the shape, which here means a reusable workflow rather than three copies. I raised it on the pull request rather than quietly widening a change a maintainer had already reviewed.

8. Check yourself