From ae08b17648e789c1046a6805ac571efb0764119b Mon Sep 17 00:00:00 2001 From: Mathieu Audebert Date: Tue, 13 Oct 2026 18:57:33 +0200 Subject: [PATCH] ci: a check of an absence in a step can fail: no command negated with ! (FRU-222) GitHub runs a step with bash -e, and bash does not stop on a command that ! inverts. The steps of the console looked for unsafe-inline and for the origin of the other worker with ! grep: with either in the policy, the step stayed green. They now exit by themselves, and workflows.test.ts fails on a negated command in any workflow. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/ci.yml | 9 ++++++--- .github/workflows/release-image.yml | 6 +++++- apps/worker/src/workflows.test.ts | 17 +++++++++++++++++ docs/decisions/image.md | 4 ++++ 4 files changed, 32 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ac726f32..f0c42bd7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -495,15 +495,18 @@ jobs: serve console-other 8083 https://other.example:8443/fruitback/ policy() { curl -s -D - -o /dev/null "http://127.0.0.1:$1/w/any/sites" | tr -d '\r' | grep -i '^content-security-policy:'; } + # WARNING: never `! grep`. Under `bash -e` a command that `!` inverts does not stop the script, + # so the step stayed green with the very thing it looks for. `absent` exits by itself. + absent() { if grep -q -- "$1" "$2"; then echo "found « $1 » in $2, where it must not be"; exit 1; fi; } policy 8082 | tee /tmp/acme policy 8083 | tee /tmp/other grep -q "connect-src 'self' https://api.acme.example;" /tmp/acme grep -q "connect-src 'self' https://other.example:8443;" /tmp/other - ! grep -q "other.example" /tmp/acme - ! grep -q "acme.example" /tmp/other + absent "other.example" /tmp/acme + absent "acme.example" /tmp/other for file in /tmp/acme /tmp/other; do grep -q "'sha256-" "$file" - ! grep -q "script-src[^;]*unsafe-inline" "$file" + absent "script-src[^;]*unsafe-inline" "$file" grep -q "frame-ancestors 'none'" "$file" done test "$(curl -s http://127.0.0.1:8082/config.js)" = 'globalThis.__FRUITBACK__ = { api: "https://api.acme.example" };' diff --git a/.github/workflows/release-image.yml b/.github/workflows/release-image.yml index b12a7820..dcb61dba 100644 --- a/.github/workflows/release-image.yml +++ b/.github/workflows/release-image.yml @@ -322,7 +322,11 @@ jobs: echo "$policy" echo "$policy" | grep -q "connect-src 'self' https://api.acme.example;" echo "$policy" | grep -q "'sha256-" - ! echo "$policy" | grep -q "script-src[^;]*unsafe-inline" + # Never `! grep`: under `bash -e` a command that `!` inverts does not stop the script. + if echo "$policy" | grep -q "script-src[^;]*unsafe-inline"; then + echo "the policy allows inline scripts" + exit 1 + fi test "$(curl -s http://127.0.0.1:8082/config.js)" = 'globalThis.__FRUITBACK__ = { api: "https://api.acme.example" };' test "$(docker exec serves id -u)" = 101 docker rm -f serves diff --git a/apps/worker/src/workflows.test.ts b/apps/worker/src/workflows.test.ts index f458e447..441dfbe0 100644 --- a/apps/worker/src/workflows.test.ts +++ b/apps/worker/src/workflows.test.ts @@ -438,6 +438,23 @@ describe('the GitHub workflows', () => { ); }); + /** + * A check that looks for an absence must be able to fail (FRU-222). GitHub runs a step with + * `bash -e`, and bash does not stop on a command that `!` inverts: `! grep -q unsafe-inline` left + * the step green with `unsafe-inline` in the policy. The form that stops is + * `if grep …; then exit 1; fi`. Raised in review. + */ + it('negate no command with `!` in a step: under `bash -e` it never fails the step', () => { + const negated = /^\s+! \S.*$/gm; + // The control: the form that was there is found. + assert.deepEqual(' run: |\n ! grep -q unsafe-inline /tmp/policy\n'.match(negated)?.length, 1); + + assert.deepEqual( + workflows().flatMap(({ file, source }) => (source.match(negated) ?? []).map((line) => `${file}: ${line.trim()}`)), + [], + ); + }); + it('publish nothing from a commit that did not pass CI', () => { const files = workflows(); const writers = files.flatMap(({ source }) => [...source.matchAll(/^ {6}\w[\w-]*: write$/gm)]); diff --git a/docs/decisions/image.md b/docs/decisions/image.md index a6e25f83..6004b82e 100644 --- a/docs/decisions/image.md +++ b/docs/decisions/image.md @@ -52,6 +52,10 @@ immutable. `packages: write`. `.github/dependabot.yml` moves an existing pin, SHA and comment together. It does not pin a new step: `workflows.test.ts` fails on any `uses:` that is not a SHA followed by its version. +- **No step negates a command with `!`** (FRU-222). GitHub runs a step with `bash -e`, and bash does + not stop on a command that `!` inverts: `! grep -q unsafe-inline` left the step green with + `unsafe-inline` in the policy of the console. A check of an absence is written + `if grep …; then exit 1; fi`, and `workflows.test.ts` fails on the other form. - **`persist-credentials: false` on every checkout** — `actions/checkout` otherwise writes the token into `.git/config`, where any later step reads it. - **Only the `publish` job holds the write permissions** (FRU-70). The workflow grants