From 0b70b9a9a3cb1d7fc430fd7b8a3acec44fe0cb77 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Sat, 25 Jul 2026 17:38:19 -0500 Subject: [PATCH 1/5] Fix claude-review's trust gate: allow same-repo PRs, not just the old fork The gate checked github.event.pull_request.head.repo.owner.login == 'jnasbyupgrade', written back when PRs came from that personal fork. Now that the convention is to push branches directly to this repo instead (never the fork), every PR's head repo owner is Postgres-Extensions, not jnasbyupgrade - so the condition never matched and claude-review silently skipped on every single PR this session (#12, #13, #14 all show it as skipped). Replaced with head.repo.full_name == github.repository, which is actually a tighter trust boundary than the old check: it only allows PRs whose head branch lives in this exact repo, requiring push access here, rather than naming one specific external fork as trusted. --- .github/workflows/claude-code-review.yml | 25 +++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) 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: From 84b21fb9158584253ef29ff697e3184fe30f478d Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Sat, 25 Jul 2026 17:47:03 -0500 Subject: [PATCH 2/5] Add pg-tle-test CI job: smoke-test count_nulls deployed purely through pg_tle Ports the smoke-test portion of cat_tools' pg-tle-test job (Postgres-Extensions/cat_tools PR #47) so count_nulls gets the same coverage: build pg_tle 1.5.2 from source, register the extension against template1 via pg_tle instead of a filesystem .control file, and verify with bin/assert_fs_clean (copied over unchanged - it's already extension-agnostic) that nothing ever lands on disk while a fresh CREATE EXTENSION count_nulls in a smoke-test db resolves and null_count() actually works. Matrix is [17, 16, 15, 14, 13, 12] for this job specifically: count_nulls' own supported range is 10-17, but pg_tle 1.5.2 only supports 12+ (pgxntool/pgtle_versions.md), so 10 and 11 are dropped here. Deliberately NOT porting cat_tools' update-path portion (TEST_EXISTING_DEPLOY=pgtle bin/test_existing update-scenario ...): count_nulls' master has no bin/test_existing at all - that infrastructure only exists on the still-unmerged extension-update-test/pgxntool-update branches. Once that lands, the update-path half of this job can be added the same way cat_tools did it. --- .github/workflows/ci.yml | 95 ++++++++++++++++++++++++++++++++++++++++ bin/assert_fs_clean | 71 ++++++++++++++++++++++++++++++ 2 files changed, 166 insertions(+) create mode 100755 bin/assert_fs_clean 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/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 "$@" From 3edd0636d26e8f478eb197f4041bba4a1a7568c1 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Wed, 29 Jul 2026 16:47:50 -0500 Subject: [PATCH 3/5] pg-tle-test: rename PGTLE_VERSION env var to avoid colliding with pgxntool's own Makefile variable pgxntool's pgtle: target reads a make variable also named PGTLE_VERSION to restrict SQL generation to one version range. Make auto-imports environment variables as make variables, so this job's own env: PGTLE_VERSION silently overrode it: make run-pgtle generated into pg_tle/1.5.2/ (the literal pinned version, passed straight through as --pgtle-version) instead of the correct range directory pg_tle/1.5.0+/, then failed when --run looked for that range directory and it didn't exist. Confirmed reproducing with a one-line `PGTLE_VERSION=1.5.2 make pgtle`, no CI or database needed. Renamed to PG_TLE_BUILD_VERSION, which doesn't collide. Filed as Postgres-Extensions/pgxntool#78 (also affects cat_tools' own pg-tle-test job, which uses the same env var name, once it upgrades pgxntool - worth a heads-up there too). --- .github/workflows/ci.yml | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1b98f33..ad41371 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -32,7 +32,18 @@ jobs: runs-on: ubuntu-latest container: pgxn/pgxn-tools env: - PGTLE_VERSION: "1.5.2" + # NOT named PGTLE_VERSION: that collides with pgxntool's own Makefile + # variable of the same name (`pgtle:`'s + # `$(if $(PGTLE_VERSION),--pgtle-version $(PGTLE_VERSION))`), which Make + # auto-imports from the environment. With that name, `make run-pgtle` + # silently generates into pg_tle/1.5.2/ (the literal version) instead of + # the correct range directory pg_tle/1.5.0+/, then fails when --run + # looks for the range directory and doesn't find it. Confirmed + # reproducing with a one-line `PGTLE_VERSION=1.5.2 make pgtle`, no CI + # needed. Filed as Postgres-Extensions/pgxntool#78 (also affects + # cat_tools' own pg-tle-test job, which uses the same env var name, once + # it upgrades pgxntool). + PG_TLE_BUILD_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 @@ -59,12 +70,12 @@ jobs: # 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 }} + - name: Build and install pg_tle ${{ env.PG_TLE_BUILD_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 + git clone --branch v${{ env.PG_TLE_BUILD_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: | From 18752fc2029b00f40eb1bcf82fdc598852abf9b9 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Wed, 29 Jul 2026 16:51:19 -0500 Subject: [PATCH 4/5] pg-tle-test: assert against EXTENSION_count_nulls_VERSION, not PGXNVERSION PR #13 (merged) flipped count_nulls.control's default_version to the stable pseudo-version, so a version-less CREATE EXTENSION now installs 'stable', not the last real release. The smoke test's version assertion still compared against PGXNVERSION (the PGXN distribution version, from META.in.json, still 1.0.0) - the exact distribution-vs-extension-version distinction RELEASE.md documents. Confirmed failing in CI (installed=stable expected=1.0.0) and locally (`make -s print-EXTENSION_count_nulls_VERSION` correctly reports stable). Fixed to assert against that variable instead. --- .github/workflows/ci.yml | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ad41371..72d50c2 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -113,7 +113,13 @@ jobs: - 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') + # EXTENSION_count_nulls_VERSION (the .control file's default_version), + # NOT PGXNVERSION (the PGXN distribution version, from META.in.json) + # -- a version-less CREATE EXTENSION installs whatever the control + # file's default_version says, and count_nulls' is currently the + # 'stable' pseudo-version, not the last real release. See + # RELEASE.md's note on distribution vs. extension versions. + EXPECTED=$(make -s print-EXTENSION_count_nulls_VERSION 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 From 06edc71d0d7b87553284cbc9e17d661f2bef86c6 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Thu, 30 Jul 2026 16:34:42 -0500 Subject: [PATCH 5/5] Restrict Claude workflows to jnasbyupgrade's own PRs/comments, not same-repo The same-repo check (head.repo.full_name == github.repository) added earlier this PR only re-established that the fork-era check had gone stale -- it didn't restore the original trust boundary. This repo has another collaborator (decibel) with push access, so any same-repo PR or @claude comment from them would still burn CLAUDE_CODE_OAUTH_TOKEN, which is tied to jnasbyupgrade's personal account. Both workflows now check the actual triggering identity instead: claude-code-review.yml on github.event.pull_request.user.login (the PR's original author, stable across synchronize events and unspoofable by PR content), and claude.yml on github.actor (whoever's comment/review/issue triggered the event) -- correct regardless of whether the PR happens to live in this repo or an external fork. --- .github/workflows/claude-code-review.yml | 24 ++++++++++++++---------- .github/workflows/claude.yml | 15 +++++++++++---- 2 files changed, 25 insertions(+), 14 deletions(-) diff --git a/.github/workflows/claude-code-review.yml b/.github/workflows/claude-code-review.yml index 090b42d..2522c44 100644 --- a/.github/workflows/claude-code-review.yml +++ b/.github/workflows/claude-code-review.yml @@ -7,13 +7,17 @@ name: Claude Code Review # 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 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. +# write-capable token. The job is gated to PRs authored by jnasbyupgrade only +# — CLAUDE_CODE_OAUTH_TOKEN is tied to their personal Claude account, and this +# repo has other collaborators (e.g. decibel) whose PRs shouldn't burn it. +# github.event.pull_request.user.login is the PR's original author and can't +# be spoofed by PR content, so this check holds regardless of whether the PR +# head lives in this repo or an external fork (an arbitrary external fork +# still can't trigger this secret-bearing job unless it's actually +# jnasbyupgrade's own fork). 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] @@ -24,11 +28,11 @@ concurrency: jobs: claude-review: - # Same-repo PRs only (never an external fork), and skip drafts (don't - # spend API/CI on unfinished PRs). + # jnasbyupgrade's own PRs only, and skip drafts (don't spend API/CI on + # unfinished PRs). if: >- github.event.pull_request.draft == false && - github.event.pull_request.head.repo.full_name == github.repository + github.event.pull_request.user.login == 'jnasbyupgrade' runs-on: ubuntu-latest timeout-minutes: 60 permissions: diff --git a/.github/workflows/claude.yml b/.github/workflows/claude.yml index c85ec00..f9059dd 100644 --- a/.github/workflows/claude.yml +++ b/.github/workflows/claude.yml @@ -14,11 +14,18 @@ on: # serializing would only delay responses and cancelling would drop them. jobs: claude: + # jnasbyupgrade only: CLAUDE_CODE_OAUTH_TOKEN is tied to their personal + # Claude account, and this repo has other collaborators (e.g. decibel) — + # without this, anyone commenting/reviewing/opening an issue with + # "@claude" would burn it. github.actor is who triggered the event + # (commenter/reviewer/issue-opener), not who owns the PR/issue it's on. if: | - (github.event_name == 'issue_comment' && contains(github.event.comment.body, '@claude')) || - (github.event_name == 'pull_request_review_comment' && contains(github.event.comment.body, '@claude')) || - (github.event_name == 'pull_request_review' && contains(github.event.review.body, '@claude')) || - (github.event_name == 'issues' && (contains(github.event.issue.body, '@claude') || contains(github.event.issue.title, '@claude'))) + github.actor == 'jnasbyupgrade' && ( + (github.event_name == 'issue_comment' && contains(github.event.comment.body, '@claude')) || + (github.event_name == 'pull_request_review_comment' && contains(github.event.comment.body, '@claude')) || + (github.event_name == 'pull_request_review' && contains(github.event.review.body, '@claude')) || + (github.event_name == 'issues' && (contains(github.event.issue.body, '@claude') || contains(github.event.issue.title, '@claude'))) + ) runs-on: ubuntu-latest timeout-minutes: 30 permissions: