Skip to content

ci-local reports PASS after a root-only assertion silently skipped, and nothing compares how CI invokes a test with how ci-local does #433

Description

@vladimirrott

scripts/ci-local.sh prints ci-local: PASS after running a test that skipped the assertion CI runs it for, and says nothing about the difference.

What happens

CI runs one release test with elevated privileges, and only one:

.github/workflows/ci.yml
  - name: Check bounded actions and removal of shell grants
    run: sudo -n bash tests/release/action-steps.test.sh

run_shell_tests discovers the same file by glob and runs every test the same way:

run_step "hygiene: ${test#"$repo_root/"}" bash "$test"

So locally it runs as whoever you are. tests/release/action-steps.test.sh carries

@unittest.skipUnless(hasattr(os, 'geteuid') and os.geteuid() == 0,
                     'real credential-drop test requires a root test subprocess')
def test_real_drop_cannot_regain_root_or_write_through_a_symlink(self):

and that is the case the sudo -n exists for. Unprivileged, it skips:

test_real_drop_cannot_regain_root_or_write_through_a_symlink ... skipped
Ran 11 tests in 0.018s
OK (skipped=1)

run_step sees exit 0, records a pass, and the summary ends:

=========================================
ci-local: PASS

The one assertion that proves the helper cannot regain root or write through a symlink never executed, and nothing in the output says so.

Why this shape and not another

required_skips already exists for exactly this problem, and is used for exactly one thing:

$ grep -n 'required_skips' scripts/ci-local.sh
79:required_skips=()
350:        required_skips+=(postgres-contract)
374:        required_skips+=(postgres-contract)
443:if ((${#required_skips[@]} > 0)); then
444:    printf 'WARNING: REQUIRED CI check(s) did not run: %s\n' "${required_skips[*]}"

A missing Postgres downgrades the run to INCOMPLETE. A missing root does not, and the reason is that the sudo step arrived after that machinery did.

The part that will not stay one test

Today the class has exactly one member. sudo -n bash tests/ matches one line in the workflows, and one test file carries a root condition. Nothing detects the next one. A second privileged step added to ci.yml gets the same silent gap on the day it lands, and the local run keeps saying PASS.

tests/release/ci-local.test.sh compares the discovered set against the workflow set by path and asserts they match. It does not compare how each side invokes them, which is where the divergence lives.

Suggested shape

Read the invocation out of the workflows rather than hardcoding a list, the way discovery already reads the file set:

  • extract tests/(release|e2e)/*.test.sh from .github/workflows/*.yml together with whether the step runs it under sudo
  • when ci-local runs one of those without being root, add it to required_skips and let the existing summary do the rest
  • extend tests/release/ci-local.test.sh so a new privileged step in a workflow fails the guard until ci-local knows about it

The point is that the guard is derived from the workflow, so it cannot go stale the way a hardcoded list does.

Reproducing

bash tests/release/action-steps.test.sh; echo "rc=$?"
# OK (skipped=1), rc=0

sudo -n bash tests/release/action-steps.test.sh 2>&1 | tail -3
# Ran 11 tests, no skip

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workinghelp wantedExtra attention is neededmediumDifficulty: needs familiarity with one subsystem

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions