Skip to content

quality.yml: gating the test tier on the security tier silently deletes all test evidence (CVE-2026-67434 disabled PHPUnit/Newman/E2E fleet-wide today) #194

Description

@rubenvdlinde

Summary

quality.yml gates the test tier on the security tier:

if: ${{ ... && !cancelled() && needs.security.result != 'failure' }}

Applied to phpunit (L1464), newman (L1720), playwright (L1985), journeydoc-capture (L2739) and sbom (L3999).

The consequence: any advisory against any dependency — including a dev-only code formatter with no runtime exposure — turns PHPUnit, Newman and E2E into skipped across the entire fleet, simultaneously.

A skipped job renders as a grey tick, not a red X. The run does not look broken. It looks fine. The tests simply did not run.

This is not hypothetical — it happened this morning across 16 repos, and it fooled an agent into filing a false "fully green" report.

Timeline (2026-08-06)

time (UTC) event
2026-08-05 23:53 CVE-2026-67434 published against squizlabs/php_codesniffer <3.13.6 (OS command injection, GHSA-hmqg-cxww-wqhq)
2026-08-06 00:17 upstream ships 3.13.6
~08:48 fleet still green — advisory not yet served by the audit path. nldesign baseline runs 110 E2E tests
~11:15 advisory begins being served. Security (composer) starts failing fleet-wide
~11:15 onward pipelinq, procest, decidesk, openbuild, openregister observed with Security (composer)=failure → test tier skipped

The trigger was not a code change in any repo. composer audit queries the live Packagist advisory database, so the fleet's test coverage was deleted by an external event, with no commit and no signal.

Note this is independent of the roave/security-advisories lockfile pin — that is a separate mechanism (an install-time conflict rule); composer audit hits the network every run.

Why this shape is dangerous

  1. It fails silent. skipped is not failure. Dashboards, gh pr checks summaries, and any tally that counts failures read zero and report success. A skipped test job is not a pass — it is the absence of a verdict, but it is rendered in the same colour family as a pass.
  2. The blast radius is fleet-wide and instantaneous. One advisory against one shared dev dependency disabled the test tier in every repo at once, without a single commit.
  3. The coupling is not justified by the risk. The CVE here is in a formatter that never runs in production. There is no scenario where "phpcs has a command-injection bug" means "we can no longer trust what PHPUnit reports."
  4. It inverts the purpose of CI. At the moment a security problem appears — precisely when you most want to know whether the code still works — the system deletes the evidence that would tell you.
  5. needs: already implies success(). This same shape has bitten repeatedly: a failing Security (npm) deleted app-versions' E2E job entirely, and any skipped producer silently deletes every dependent gate downstream. The !cancelled() guards were added precisely because of this class of bug; the needs.security.result != 'failure' clause reintroduces it deliberately.

What is not in question

A security failure should block a merge. That is not the issue. The issue is whether it should also delete the tests — two different questions currently answered by one conditional.

Options (for humans to choose — deliberately not choosing one)

Option A — decouple entirely. Drop needs.security.result != 'failure' from the test jobs. Let tests always run; enforce security at the merge gate via branch protection (require Security (composer) to pass). Tests always produce a verdict; security still blocks merge. Cost: a few CI minutes spent on a PR that cannot merge anyway.

Option B — decouple by severity. Run tests regardless; only gate on critical/high advisories. Requires the security job to emit a severity output rather than a bare pass/fail. Note this specific advisory has severity: null from the source feed, so the fallback for unknown severity must be chosen explicitly.

Option C — decouple by ecosystem scope. Gate on packages (runtime) advisories only, never packages-dev. A dev-only formatter CVE would not touch the test tier; a runtime advisory still would. Closest to intent, needs composer audit --no-dev as a second signal.

Option D — keep the gate, fix the rendering. Leave behaviour as-is but make a security-induced skip loud: have the Quality Report job hard-fail with an explicit "TEST TIER NOT EXECUTED — no verdict" message whenever it observes security=failure alongside skipped test jobs. Does not restore coverage, but removes the silent-green.

Option E — A + D. Tests always run; the report distinguishes "tests passed, security failed" from "tests never ran".

Recommendation on process, not on option

Whichever option is chosen, add an invariant to the Quality Report: skipped in the test tier must never be summarised as a pass. The report should state, in words, that no verdict exists. The failure mode that cost us this morning was not the gate — it was that the absence of a verdict was indistinguishable from a good one.

Immediate remediation (already in flight, separate from this issue)

squizlabs/php_codesniffer bumped 3.13.53.13.6 in all 16 repos. Verified A/B against the real lockfiles:

php_codesniffer 3.13.5 → composer audit --locked → EXIT=1, CVE-2026-67434
php_codesniffer 3.13.6 → composer audit --locked → EXIT=0, no advisories

No waivers, no continue-on-error, no ignore entries were used anywhere — those would have made the security job green while leaving the test tier gated on the next advisory.

The bump closes today's instance. It does not close the design question, which is why this issue exists. The next advisory against any shared dev dependency will do exactly the same thing.

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