diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 23bacab..1b98f33 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -19,3 +19,98 @@ jobs: uses: actions/checkout@v4 - name: Test on PostgreSQL ${{ matrix.pg }} run: pg-build-test + + pg-tle-test: + strategy: + matrix: + # Intersection of count_nulls' own supported range (10-17, see the + # `test` job above) with pg_tle 1.5.2's supported range (12-18, see + # pgxntool/pgtle_versions.md): drop 10 and 11 since pg_tle doesn't + # support them. + pg: [17, 16, 15, 14, 13, 12] + name: 🧩 pg_tle ${{ matrix.pg }} + runs-on: ubuntu-latest + container: pgxn/pgxn-tools + env: + PGTLE_VERSION: "1.5.2" + steps: + # A dedicated cluster, never shared with the other jobs in this + # workflow: pg_tle requires shared_preload_libraries and mixing + # pg_tle/non-pg_tle extension installs on one cluster can misbehave. + - name: Start PostgreSQL ${{ matrix.pg }} + run: pg-start ${{ matrix.pg }} + - name: Check out the repo + uses: actions/checkout@v4 + - name: Install rsync + run: apt-get install -y rsync + - name: Install pgtap (test harness dependency) + # pgTAP is a filesystem-installed dependency of the TEST HARNESS, not + # part of what this job proves is pg_tle-only -- it's not being + # deployed via pg_tle here, and never will be. Installing it + # explicitly here, before the baseline snapshot, makes it part of the + # accepted starting state -- like any other extension already on + # disk -- instead of needing a hardcoded exclude-by-name rule that + # would erode the whole point of diffing against a baseline. + run: make pgtap + - name: Snapshot filesystem extension control files (pre-pg_tle baseline) + # Whatever ships on disk by default (e.g. contrib, and now pgtap) + # before we install pg_tle. bin/assert_fs_clean's later checks diff + # against this, so they flag ANY extension that lands on disk instead + # of being registered via pg_tle -- not just count_nulls -- without + # hardcoding contrib/pgtap names. + run: bin/assert_fs_clean snapshot ${{ matrix.pg }} /tmp/control_baseline.txt + - name: Build and install pg_tle ${{ env.PGTLE_VERSION }} + # flex/bison/libkrb5-dev aren't in the pgxn-tools image; pg_tle's build + # needs them (guc-file.l, and clientauth.c includes gssapi.h). + run: | + apt-get install -y flex bison libkrb5-dev + git clone --branch v${{ env.PGTLE_VERSION }} --depth 1 https://github.com/aws/pg_tle.git /tmp/pg_tle + make -C /tmp/pg_tle install + - name: Enable pg_tle and restart PostgreSQL ${{ matrix.pg }} + run: | + echo "shared_preload_libraries = 'pg_tle'" >> /etc/postgresql/${{ matrix.pg }}/test/postgresql.conf + pg_ctlcluster ${{ matrix.pg }} test restart + pg_isready -t 30 + - name: Register pg_tle + count_nulls against template1 + # template1, not the ambient default db: pg_tle's registration catalog + # is per-database, and `createdb` only inherits it because it copies + # template1 by default. Every count_nulls database used below (the + # smoke-test db) is created AFTER this step specifically so it + # inherits both registrations. + run: | + psql -d template1 -c "CREATE EXTENSION pg_tle" + PGDATABASE=template1 make run-pgtle + - name: Verify no stray extension control files landed on the filesystem + # CRITICAL, and intentionally redundant with the count_nulls-specific + # check in the next step: a filesystem control file silently wins + # over a pg_tle-registered extension of the same name, which would + # make this whole job a false pass without ever raising an error. Run + # again after every step below that could plausibly write extension + # files to disk -- never trust a single check to catch everything. + run: bin/assert_fs_clean verify ${{ matrix.pg }} /tmp/control_baseline.txt + - name: Install count_nulls purely via pg_tle (fresh install, no filesystem trace) + # count_nulls is never `make install`ed in this job, so a successful + # CREATE EXTENSION here can only be resolving through pg_tle's + # registration, not a control file on disk. Checked explicitly here + # too (not just via the comprehensive check above) as a guard + # specifically for the extension under test, in case that check's + # exclude-list logic has a bug. + run: | + test ! -e /usr/share/postgresql/${{ matrix.pg }}/extension/count_nulls.control + createdb count_nulls_smoke + psql -d count_nulls_smoke -c "CREATE EXTENSION count_nulls" + - name: Verify count_nulls works when deployed via pg_tle + run: | + INSTALLED=$(psql -d count_nulls_smoke -tAc "SELECT extversion FROM pg_extension WHERE extname = 'count_nulls'") + EXPECTED=$(make -s print-PGXNVERSION 2>/dev/null | sed -n 's/.*set to "\(.*\)"$/\1/p') + echo "installed=$INSTALLED expected=$EXPECTED" + if [ -z "$INSTALLED" ] || [ -z "$EXPECTED" ] || [ "$INSTALLED" != "$EXPECTED" ]; then + echo "FAIL: installed='$INSTALLED' expected='$EXPECTED'"; exit 1 + fi + RESULT=$(psql -d count_nulls_smoke -v ON_ERROR_STOP=1 -tAc "SELECT null_count(1, NULL, 2)") + echo "null_count(1, NULL, 2)=$RESULT" + if [ "$RESULT" != "1" ]; then + echo "FAIL: expected null_count(1, NULL, 2) = 1, got '$RESULT'"; exit 1 + fi + - name: Verify no stray extension control files after the fresh-install smoke test + run: bin/assert_fs_clean verify ${{ matrix.pg }} /tmp/control_baseline.txt diff --git a/.github/workflows/claude-code-review.yml b/.github/workflows/claude-code-review.yml index fc7e26c..090b42d 100644 --- a/.github/workflows/claude-code-review.yml +++ b/.github/workflows/claude-code-review.yml @@ -1,16 +1,19 @@ name: Claude Code Review # Runs on PRs INTO this repo. We use pull_request_target (not pull_request) so -# that PRs from a fork can access CLAUDE_CODE_OAUTH_TOKEN — GitHub withholds -# secrets from `pull_request` runs triggered by forks, which is why the plain -# `pull_request` version never worked for fork PRs. +# that the workflow definition always comes from the base branch, never from +# the PR's own head - a plain `pull_request` run uses whatever workflow file +# is on the PR branch itself, which a same-repo feature branch could modify to +# steal secrets before this ever runs. # # SECURITY: pull_request_target runs in the BASE repo with secrets and a -# write-capable token. The job is gated to PRs from the trusted `jnasbyupgrade` -# fork only — an arbitrary external fork can never trigger this secret-bearing -# job. The workflow file always comes from the base branch (master), so a PR -# cannot modify the reviewer that runs on it. We check out the PR head only for -# read context (persist-credentials: false) and never build or execute PR code. +# write-capable token. The job is gated to PRs whose head branch lives in THIS +# repo (not an external fork) — that requires push access to this repo in the +# first place, so an arbitrary external fork can never trigger this +# secret-bearing job. The workflow file always comes from the base branch +# (master), so a PR cannot modify the reviewer that runs on it. We check out +# the PR head only for read context (persist-credentials: false) and never +# build or execute PR code. on: pull_request_target: types: [opened, synchronize, reopened, ready_for_review] @@ -21,11 +24,11 @@ concurrency: jobs: claude-review: - # Trusted fork only, and skip drafts (don't spend API/CI on unfinished PRs). - # To add more trusted owners, extend the head-owner check. + # Same-repo PRs only (never an external fork), and skip drafts (don't + # spend API/CI on unfinished PRs). if: >- github.event.pull_request.draft == false && - github.event.pull_request.head.repo.owner.login == 'jnasbyupgrade' + github.event.pull_request.head.repo.full_name == github.repository runs-on: ubuntu-latest timeout-minutes: 60 permissions: diff --git a/bin/assert_fs_clean b/bin/assert_fs_clean new file mode 100755 index 0000000..80fcc91 --- /dev/null +++ b/bin/assert_fs_clean @@ -0,0 +1,71 @@ +#!/usr/bin/env bash +# +# assert_fs_clean - Verify no stray PostgreSQL extension control files exist on +# disk, to prove an extension was deployed purely via pg_tle (not filesystem +# install). Extracted from the pg-tle-test CI job so the SAME check can run at +# every checkpoint that could plausibly write extension files to disk +# (registration, after an update, after a binary pg_upgrade, ...), instead of +# being duplicated inline in ci.yml or trusted to a single check at the end. +# +# A pre-existing filesystem control file silently wins over a pg_tle-registered +# extension of the same name -- PostgreSQL never reports an error, it just +# quietly resolves CREATE EXTENSION from disk instead of pg_tle's catalog. That +# makes "prove pg_tle-only" a real, load-bearing assertion, not a formality: it +# must run AFTER whatever it's guarding, not just before, since the whole point +# is confirming nothing wrote to disk THROUGHOUT the guarded flow, not merely +# that the environment started clean. +# +# USAGE: bin/assert_fs_clean [args] +# +# snapshot PG_MAJOR BASELINE_FILE +# Record the current *.control files in PG_MAJOR's extension directory to +# BASELINE_FILE. Run this BEFORE installing pg_tle (or anything else), so +# whatever ships on disk by default (e.g. contrib) is excluded +# automatically -- no hardcoded exclude list to keep in sync. +# +# verify PG_MAJOR BASELINE_FILE +# Fail if any *.control file exists now that wasn't in BASELINE_FILE, +# other than pg_tle.control itself (the one legitimate filesystem install +# in this flow). Run this after EVERY step that could plausibly have +# written extension files to disk. +set -euo pipefail + +extdir_of() { echo "/usr/share/postgresql/$1/extension"; } + +snapshot() { + local pg_major=$1 baseline=$2 + find "$(extdir_of "$pg_major")" -maxdepth 1 -name '*.control' | sort > "$baseline" +} + +verify() { + local pg_major=$1 baseline=$2 after new + after=$(mktemp) + find "$(extdir_of "$pg_major")" -maxdepth 1 -name '*.control' | sort > "$after" + new=$(comm -13 "$baseline" "$after" | grep -vx '.*/pg_tle\.control' || true) + rm -f "$after" + if [ -n "$new" ]; then + echo "FAIL: unexpected extension control file(s) on disk (everything but pg_tle must be registered via pg_tle, not filesystem-installed):" >&2 + echo "$new" >&2 + exit 1 + fi + echo "OK: no stray extension control files on disk (PG $pg_major)" +} + +usage() { + echo "usage: bin/assert_fs_clean [args]" >&2 + echo " snapshot PG_MAJOR BASELINE_FILE" >&2 + echo " verify PG_MAJOR BASELINE_FILE" >&2 + exit 2 +} + +main() { + local cmd=${1:-} + shift || true + case "$cmd" in + snapshot) snapshot "$@" ;; + verify) verify "$@" ;; + *) usage ;; + esac +} + +main "$@"