Ask
28

Three checks in one pipeline had been silently dead for months, and every one of them looked like it was passing

Audited a publish pipeline after a bad release and found three separate things that could not fail. Different causes, one shape.

1. A secret with an invisible byte order mark. Written from PowerShell as "value" | wrangler secret put, which prepends a UTF-8 BOM. OAuth broke immediately and silently, and the only visible trace was in a request log: client_id=%EF%BB%BF560593.... Writing it from bash with printf '%s' 'value' | ... fixes it. Nothing anywhere reports a BOM.

2. A parser that matched a warning. The script pulled JSON out of a CLI by scanning for the first [, which happily matched [WARNING]. When the query failed, the resulting map came back empty, and an empty map made a near-duplicate filter pass everything through. A silent yes instead of a loud no.

3. A check whose failure looked exactly like success. The integrity query was passed as --command "<sql>" with the SQL starting on the next line. The current CLI rejects both the space-separated flag and a value beginning with a newline. The output went straight into grep, so the rejection produced no matching lines, which is precisely what a clean run looks like. Every release printed an empty integrity section and nobody reads an empty section as a failure.

Any check whose success and whose failure look the same is not a check. All three passed a code review.

10 answers Share
Report

Answering anonymously, a moderator will review it first.

  • @green_but_dead · 2w ago · 3 replies

    Adding the version of this that got me: a test suite that ran zero tests and reported success. A glob pattern stopped matching after a directory rename, the runner found no files, exited 0, and the badge stayed green for five weeks.

    Now every suite asserts a minimum test count. If it runs fewer than N tests, it fails. Sounds paranoid and it has caught the same thing twice since.

    24
    Share
    Reply

    Answering anonymously, a moderator will review it first.

    Report
    • @prove_it_fails · 2w ago

      That is the generalisation: make the check assert something about itself, not only about the thing it is checking. Minimum test count, non-empty output, a known-bad input that must be rejected.

      16
      Share
      Reply

      Answering anonymously, a moderator will review it first.

      Report
    • @exit_codes_matter · 2w ago

      A canary input in the test data that is supposed to fail is the cheapest version and almost nobody does it.

      10
      Share
      Reply

      Answering anonymously, a moderator will review it first.

      Report
  • @invisible_bom · 2w ago · 2 replies

    On the BOM specifically, because it wastes days: it is the default on several Windows redirect and encoding paths, not an exotic accident. If a secret, a key file or a config gets written on Windows and read on Linux, assume the BOM until you have checked.

    file will tell you. xxd | head -1 will tell you. The error message from whatever consumes it will not, ever.

    21
    Share
    Reply

    Answering anonymously, a moderator will review it first.

    Report
    • @grep_bit_me · 2w ago

      It also survives copy and paste through a surprising number of tools, which is how it gets into a secret store in the first place.

      1
      Share
      Reply

      Answering anonymously, a moderator will review it first.

      Report
  • @exit_codes_matter · 2w ago · 3 replies

    Number two and three are both the same underlying bug and it has a name: you piped a command into something that does not care whether it succeeded.

    set -o pipefail catches the whole class. Without it, the exit status of a pipeline is the status of the last command, so a failing tool piped into a happy grep is a success as far as the shell is concerned. With set -euo pipefail at the top of the file, both of those stop silently and start loudly.

    It is one line and it is the difference between a script that reports and a script that performs.

    27
    Share
    Reply

    Answering anonymously, a moderator will review it first.

    Report
    • @prove_it_fails · 2w ago

      The pipeline now captures into a variable and checks it separately, but pipefail would have caught the original version on day one.

      15
      Share
      Reply

      Answering anonymously, a moderator will review it first.

      Report
    • @grep_bit_me · 2w ago

      grep also exits non-zero when it finds nothing, which people then work around with || true, which reintroduces the whole problem.

      11
      Share
      Reply

      Answering anonymously, a moderator will review it first.

      Report
  • @green_but_dead · 2w ago

    Five weeks of a green badge on a suite that ran nothing. I still think about it.

    1
    Share
    Reply

    Answering anonymously, a moderator will review it first.

    Report
  • @invisible_bom · 2w ago

    Three bytes. Two days.

    1
    Share
    Reply

    Answering anonymously, a moderator will review it first.

    Report