DEV Community

omarmohelal
omarmohelal

Posted on Fully Autonomous

I pointed my own security tool at my own GitHub Action. It found two bugs.

I write an application-security tool called SecHelix. Last week I added a GitHub
Action to it. Before merging, I pointed the tool at its own new Action.

It found two real defects. Neither would have failed a test. Both were the kind
of thing I would have shipped.

This is a post about those two bugs, because they are better arguments for
evidence-first review than anything I could write about the methodology.

Bug one: the artifact was the wrong copy

The Action runs an audit and uploads the result as a build artifact so you can
download it from the workflow run. The step looked like this:

- run: sechelix audit "$PATH" --json > sechelix-run.json
- uses: actions/upload-artifact@...
  with:
    path: sechelix-run.json
Enter fullscreen mode Exit fullscreen mode

Obvious. Works. Ships.

Here is what I had forgotten about my own codebase. The runner writes two copies
of every result. The one it persists to disk goes through storage.write_json,
which runs the payload through a redactor. The one --json prints to stdout is
result.to_dict(), raw.

I only had to run the redactor against a payload to see it:

persisted (storage.write_json):  "authorization": "[REDACTED]"
stdout    (audit --json):        "authorization": "Bearer sk-live-abc123"
Enter fullscreen mode Exit fullscreen mode

So the Action was uploading the unredacted projection into a build artifact that
anyone with read access to the repository can download, for the seven days it
lives. During a security review, of all things — the exact run most likely to
have a secret quoted in a node payload, because that is what it was looking for.

The fix is one line: re-read the persisted copy with
sechelix report --format json before uploading anything.

The interesting part is not the fix. It is that there were two projections of
the same object with different safety properties, and only one of them was
documented as redacted.
No test could have caught this, because both
projections were behaving exactly as written. The defect was in the seam.

Bug two: an output value could forge another output

GitHub Actions steps communicate through a file:

echo "outcome=BLOCKED" >> "$GITHUB_OUTPUT"
Enter fullscreen mode Exit fullscreen mode

It is newline-delimited. So a value containing a newline writes a second key.
And when a key appears twice, the runner takes the last one.

Which means a value like this:

RUN-X
outcome=PASS
blocking-count=0
Enter fullscreen mode Exit fullscreen mode

produces:

outcome=BLOCKED
reason=1 verified finding(s) at CRITICAL or HIGH severity are open.
run-id=RUN-X
outcome=PASS          <-- forged
blocking-count=0      <-- forged
incomplete=false
blocking-count=1
Enter fullscreen mode Exit fullscreen mode

outcome=PASS wins. A gate that said BLOCKED reports PASS.

Now — the honest version. In this Action's actual flow, run_id is generated by
the runner and constrained by a regex that cannot contain a newline. This was
not a reachable bypass.
I have seen this class of thing written up as critical
by tools that never checked reachability, and it is exactly the kind of finding
that costs a reviewer an afternoon and costs the tool its credibility.

So it is reported at its real severity: defence in depth, fixed because the fix
is four lines and it makes the property hold for every field instead of for
one field by accident.

I wrote the regression test first and confirmed it red:

AssertionError: 7 != 5 : ['outcome=BLOCKED', ..., 'outcome=PASS', ...]
Enter fullscreen mode Exit fullscreen mode

Why this is the whole argument

Most security tooling optimises for finding more. The expensive failure in
AppSec is the opposite: a queue of confident findings where a third are wrong,
no cheap way to tell which third, and after the second bad one nobody reads the
third.

So SecHelix is built the other way round. Every candidate goes to an independent
verifier whose job is to disprove it, and the report includes what it refuted
and why. A run that could not analyse anything returns INCOMPLETE, never a
clean PASS.

Both bugs above are that idea working. Bug one survived refutation and got
reported at Medium with a reproduction. Bug two survived as a real defect but
failed the reachability test, and got reported as defence in depth instead of
as a critical gate bypass. The second outcome is the one I care about, because
anyone can build a tool that shouts.

Try the version that takes 90 seconds

There is a demo in the repo that is small enough to read in one sitting. Two
candidates in a small multi-tenant expense API:

git clone https://github.com/omarmohelal/SecHelix && cd SecHelix
python examples/expense-api/prove.py
Enter fullscreen mode Exit fullscreen mode

Python 3.10+, no dependencies, no network, no containers.

The one every pattern matcher flags builds a SQL ORDER BY with an f-string.
It is not exploitable. The sort key is mapped onto one of five module constants,
and the proof checks by identity that the object reaching the query text is
that constant — so it is not "the payloads I tried were rejected", it is "no
string derived from the request can survive the mapping".

The one scanners walk past is three lines below. The endpoint has an
authorization check. It is correct. An employee token really does get a 403. And
it asks "may someone like you open a receipt?" instead of "is this receipt
yours?", over a query scoped by nothing — so any approver at any tenant reads any
receipt by id.

That is the difference in one screen: a check that is present and answers the
wrong question is invisible to a tool looking for a missing check.

The demo goes through the whole loop — reproduce, root cause, a two-line fix,
and a regression test that was red before the fix. CI drives it in all three
states on every run (vulnerable, patched, reverted), so the README cannot drift
into describing something that no longer happens.

What I am not claiming

There is no detection-rate number, and there is not going to be one from me.
The public benchmark position is NOT_MEASURED, deliberately — I am not going to
grade my own homework and publish the score.

If you want to score it, or anything else, there is a separate
challenge repo: ten cases,
three of them decoys, a false positive costs exactly what a miss costs, UNKNOWN
scores zero rather than counting as an error, and the ground truth is public and
arguable. It is deliberately tool-neutral. If it makes SecHelix look bad, that is
information too.


SecHelix is Apache-2.0 and works as an Agent Skill with no runtime required:

npx skills@latest add omarmohelal/SecHelix --skill sechelix
Enter fullscreen mode Exit fullscreen mode

If it is useful in your security workflow, a star on
the repository helps other people
find it.

Top comments (0)