Skip to content

[BUG]: Workflow hardening guards only cover two of the four workflows #125

Description

@dberzan

What happened?

tests/test_ci_workflows.py encodes two CI hardening invariants as tests — every action pinned to a commit SHA, and no checkout persisting credentials. Both iterate a hardcoded list:

# tests/test_ci_workflows.py:228 and :234
for workflow_name in ("ci.yml", "security.yml"):

Expected: the guards protect the invariant across .github/workflows/.
Actual: they protect it in two files. dco.yml and publish-docs.yml are never loaded, and both have drifted from the invariant the tests exist to hold.

workflow every action SHA-pinned permissions: declared persist-credentials: false
ci.yml yes yes 10 / 10 checkouts
security.yml yes yes 4 / 4 checkouts
dco.yml yes yes 0 / 1
publish-docs.yml noactions/checkout@v4 no 0 / 1

35 of the 36 uses: references in the repository are SHA-pinned with a version comment. The one exception is publish-docs.yml:14 — and it is in the only workflow that puts a secret in a job environment:

      - name: Install Fern
        run: npm install -g fern-api          # unpinned, no lockfile

      - name: Publish Docs
        env:
          FERN_TOKEN: ${{ secrets.FERN_TOKEN }}

With no permissions: block that job also inherits the repository default GITHUB_TOKEN scope, and without persist-credentials: false the token is written to .git/config and stays readable by every later step in the job.

I am filing this as a bug rather than through SECURITY.md deliberately. There is no proof of concept and nothing demonstrably exploitable — the workflow triggers only on push to main, so it is not reachable from a fork PR. The defect I am reporting is that the guard tests do not cover what they claim to; the token placement is why that gap is worth closing rather than leaving. Happy to move this to the private channel if you would rather have it there.

Reproduction steps

The guards pass today with an unpinned action in the tree. Demonstrate the hole by unpinning a file the guards do cover, and comparing:

# 1. The invariant is currently satisfied — the guards are green.
python -m pytest tests/test_ci_workflows.py -q          # 11 passed

# 2. publish-docs.yml already violates it, and no test notices.
grep -n 'uses: actions/checkout@' .github/workflows/publish-docs.yml
#    .github/workflows/publish-docs.yml:14:        uses: actions/checkout@v4
grep -c 'persist-credentials: false' .github/workflows/publish-docs.yml   # 0
grep -c '^permissions:' .github/workflows/publish-docs.yml                # 0
python -m pytest tests/test_ci_workflows.py -q          # still 11 passed

# 3. Make the same edit in a covered file and the guard fires, confirming
#    the difference is coverage, not the rule.
sed -i '' 's|actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0|actions/checkout@v4|' .github/workflows/ci.yml
python -m pytest tests/test_ci_workflows.py -q          # 1 failed, 10 passed
git checkout .github/workflows/ci.yml

SkillEvaluator version or commit

ea9b23971f53f3123382d37ebca13c91753f8d24 (v0.2.1, main)

Environment

Python 3.13.12, macOS 15 (arm64), uv sync --extra all --extra dev. Not environment-specific — the guards are static YAML assertions.

Proposed fix

Implemented and green locally; PR to follow.

  1. Replace the hardcoded workflow tuple in both guards with a glob over .github/workflows/, so a workflow added tomorrow is covered the day it lands. This is the part that prevents recurrence.
  2. Add a guard that every workflow declares permissions: and never uses write-all.
  3. Fix what those guards then surface — publish-docs.yml gets the 9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 checkout pin already used 14 times in this repo, permissions: contents: read, and persist-credentials: false; dco.yml gets persist-credentials: false.

Two things worth your input, both found while implementing:

  • The Fern CLI version. My first attempt hardcoded npm install -g fern-api@<version> here. That was wrong: fern/fern.config.json already pins the CLI, and ci.yml's docs lane derives the version from it to run fern check — a second hardcoded pin would let validation and publishing drift onto different versions. The PR instead derives it exactly as ci.yml does. Flagging it because the same trap is easy to fall into again.
  • Scope of the permissions guard. It asserts the scope is declared and never write-all, at workflow and job level. It deliberately still allows granular writes like contents: write, since a release workflow will legitimately need one. Say the word if you want it stricter.

Before submitting

  • I agree to follow this project's Code of Conduct
  • I have searched the open issues and found no duplicate of this report

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions