Skip to content

A disabled analyser leg publishes a green check named after the analyser — the tool's absence reads as its success #146

Description

@rubenvdlinde

The shape

.github/workflows/quality.yml, job php-quality, builds one matrix leg per tool unconditionally:

matrix:
  tool: [lint, phpcs, phpmd, psalm, phpstan, phpmetrics]

and each optional leg opts out inside the step:

psalm)
  if [ "${{ inputs.enable-psalm }}" != "true" ]; then
    echo "Psalm is disabled — skipping."
    exit 0
  fi

exit 0 means the job succeeds. GitHub then publishes a check context literally named quality / PHP Quality (psalm) with a green tick, for a run in which psalm was never invoked. Same for phpcs, phpmd, phpstan, phpmetrics.

This is the falsely-green variant of a dead gate: a check named after a tool is asserting the tool's absence, and it looks exactly like the tool passing. Anyone reading the PR — human or bot — sees five analysers reporting clean.

Measured

ConductionNL/nldesign, whose caller had enable-psalm/phpstan/phpcs: false:

run job conclusion log
30879432992 91897402679 PHP Quality (psalm) success (18s) Psalm is disabled — skipping.
30889958278 91929497397 PHP Quality (psalm) success (18s) Scanning files… Analyzing files… No errors found! Checks took 5.14 seconds

Two identical green ticks, ~identical wall time, opposite meanings. Only the log distinguishes them.

Note the wall time is not a tell: the whole job is checkout + setup-php + cached composer install, and psalm itself is 5s on this repo. You cannot spot the fake one from the checks list, the job duration, or the conclusion.

Not fixed here, deliberately

nldesign was fixed by turning the three tools on (ConductionNL/nldesign#206) — they all pass there, so the honest gate was free. That fixes one repo, not the shape.

The shape needs a fleet decision, because both obvious remedies change the check-context name set, and any repo with a branch-protection rule requiring quality / PHP Quality (psalm) would then hang on "Expected — waiting for status to be reported" forever:

  1. Build the matrix from the enabled tools (fromJSON over the enable-* inputs). Disabled tools then emit no context at all — honest absence rather than false presence.
  2. Label the leg (PHP Quality (psalm — not enabled)) via a matrix include carrying a computed label. Keeps a visible row, still changes the name.

Either way this needs an audit of required contexts across the fleet before it lands, plus a caller-side sweep — several repos disable analysers whose configs are present and passing, exactly as nldesign did.

Related

The same "absence looks like success" family already fixed in this file: #121 (composer <script> exits 1 for both "ran and failed" and "script not defined"), #138 (coverage-baseline gate reporting success while doing nothing), #140 (a phpmd finding silently deleting the PHPUnit signal).

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