diff --git a/.github/workflows/self-test.yml b/.github/workflows/self-test.yml index 961d903..4349624 100644 --- a/.github/workflows/self-test.yml +++ b/.github/workflows/self-test.yml @@ -19,6 +19,19 @@ jobs: runs-on: '["ubuntu-latest"]' yamllint-paths: ".github/ .yamllint.yml" + # The plaintext-secret scan in sops-audit.yml reaches every consumer repo + # through the floating @v2 tag, and its allowlist can only make the scan + # LESS sensitive — the direction that fails silently. Half these fixtures + # are real secrets that must still be caught, so widening the allowlist + # carelessly turns this red instead of quietly turning detection off. + plaintext-scan: + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + - name: Plaintext-secret scan fixtures + run: ./tests/plaintext-scan.test.sh + composite-actions-syntax: runs-on: ubuntu-latest timeout-minutes: 10 diff --git a/.github/workflows/sops-audit.yml b/.github/workflows/sops-audit.yml index c05e028..fdc0a9d 100644 --- a/.github/workflows/sops-audit.yml +++ b/.github/workflows/sops-audit.yml @@ -85,7 +85,38 @@ jobs: # ERE that matches common secret-assignment forms in config files. # Anchors on the key name so prose hits like "this is a token for X" # are excluded by the [:=] requirement. - pattern='(password|passwd|secret|api[_-]?key|token|private[_-]?key|access[_-]?key)[[:space:]]*[:=][[:space:]]*["'"'"']?[A-Za-z0-9/_+.=-]{8,}' + KEY='(password|passwd|secret|api[_-]?key|token|private[_-]?key|access[_-]?key)' + SEP='[[:space:]]*[:=][[:space:]]*' + Q='["'"'"']?' + pattern="${KEY}${SEP}${Q}[A-Za-z0-9/_+.=-]{8,}" + + # --- Allowlist ------------------------------------------------- + # The pattern above has no notion of what a value MEANS, so + # ordinary configuration trips it: `update_password: on_create` is + # the documented API of ansible.builtin.user, and `on_create` + # clears the 8-character floor. Consumers cannot reword task code + # around it, and their only lever was excluding a whole file from + # scanning — disarming real detection to silence one line. + # + # Two classes are suppressed, both cases where the value CANNOT be + # a secret: + # + # 1. Indirection — the value names something else rather than + # being the thing. Jinja/Go templates, shell and env + # expansion, Ansible vault refs, lookups, and SOPS ciphertext. + IND="${KEY}${SEP}${Q}([{][{]|[$][{]|[$][A-Za-z_]|ENC[[]|!vault|lookup[(])" + # + # 2. Config enums — a closed set of literals that are values of + # a setting, not credentials. + ENUMS='on_create|always|never|absent|present|true|false|yes|no|none|null|enabled|disabled|required|optional|latest|stable|default|auto|ignore|prompt' + ENUM="${KEY}${SEP}${Q}(${ENUMS})${Q}[[:space:]]*(#.*)?$" + # + # Deliberately NOT allowlisted: changeme, placeholder, secret, + # password and friends. Those are plausible real bad values and + # must keep failing. The allowlist only ever suppresses; it never + # widens what counts as a secret. + # + # See https://github.com/nkg/github-actions/issues/74 any=0 while IFS= read -r file; do # Skip via case-globs from input. @@ -102,13 +133,22 @@ jobs: *.env|*.yml|*.yaml|*.json|*.toml) ;; *) continue ;; esac - # Don't flag SOPS-encrypted files (the ciphertext can look secret-ish). - if grep -q -E '^sops:|^sops_|"sops"' "$file" 2>/dev/null; then + # Don't flag SOPS-encrypted files (the ciphertext can look + # secret-ish). `[sops]` is the TOML form of the metadata block; + # without it an encrypted .toml was scanned rather than skipped, + # even though .toml is in the extension list above. + if grep -q -E '^sops:|^sops_|"sops"|^\[sops\]' "$file" 2>/dev/null; then continue fi - if grep -E -i -q "$pattern" "$file" 2>/dev/null; then + # Match, then drop the allowlisted forms. Reported from the same + # filtered list that decides pass/fail, so what you are shown is + # exactly what failed the build. + hits=$(grep -E -i -n "$pattern" "$file" 2>/dev/null \ + | grep -v -E -i "$IND" \ + | grep -v -E -i "$ENUM" || true) + if [ -n "$hits" ]; then echo "::error file=$file::potential plaintext secret" - grep -E -i -n "$pattern" "$file" | head -5 + printf '%s\n' "$hits" | head -5 any=1 fi done < <(git ls-files) diff --git a/tests/plaintext-scan.test.sh b/tests/plaintext-scan.test.sh new file mode 100755 index 0000000..5e9d0aa --- /dev/null +++ b/tests/plaintext-scan.test.sh @@ -0,0 +1,79 @@ +#!/usr/bin/env bash +# +# Test the plaintext-secret scan in .github/workflows/sops-audit.yml. +# +# This workflow is consumed by every repo in the org through a floating @v2 +# tag, so a change here reaches all of them at once. The allowlist added for +# issue #74 can only ever make the scan LESS sensitive, which is exactly the +# direction that fails silently — a scan that stops detecting reports success. +# +# So: the fixtures below are half true-positives that must still fail. If the +# allowlist is ever widened carelessly, those go green and this test goes red. +# +# The patterns are extracted FROM the workflow rather than restated here. +# A copy would drift, and a drifted test proves nothing about what runs. +# SC2153/SC2154: KEY/SEP/Q/ENUMS come from the eval below, which shellcheck +# cannot follow — that indirection is the point, so the test cannot drift +# from the workflow it is testing. +# SC2016: the fixtures deliberately contain literal $VAR and ${VAR}. Single +# quotes are correct; expanding them would destroy the case under test. +# shellcheck disable=SC2153,SC2154,SC2016 + +set -uo pipefail + +WF=".github/workflows/sops-audit.yml" +[ -f "$WF" ] || { echo "run from the repo root" >&2; exit 1; } + +# Pull the four assignments out of the run: block verbatim. +eval "$(grep -E "^ +(KEY|SEP|Q|ENUMS)=" "$WF" | sed 's/^ *//')" + +pattern="${KEY}${SEP}${Q}[A-Za-z0-9/_+.=-]{8,}" +IND="${KEY}${SEP}${Q}([{][{]|[$][{]|[$][A-Za-z_]|ENC[[]|!vault|lookup[(])" +ENUM="${KEY}${SEP}${Q}(${ENUMS})${Q}[[:space:]]*(#.*)?$" + +scan() { # 0 = flagged, 1 = clean + printf '%s\n' "$1" \ + | grep -E -i "$pattern" \ + | grep -v -E -i "$IND" \ + | grep -v -E -i "$ENUM" \ + | grep -q . +} + +pass=0; fail=0 +check() { #