Skip to content

Add CI workflows, /ci skill, and test fixes#22

Open
jnasbyupgrade wants to merge 17 commits into
masterfrom
add-ci
Open

Add CI workflows, /ci skill, and test fixes#22
jnasbyupgrade wants to merge 17 commits into
masterfrom
add-ci

Conversation

@jnasbyupgrade
Copy link
Copy Markdown
Contributor

Summary

  • Add GitHub Actions CI workflow (.github/workflows/ci.yml) — matrix of PG 12–17, resolves paired pgxntool branch by name, pre-installs pgtap to prevent concurrent install race condition
  • Add /ci skill for monitoring CI runs after every push (shell-driven, BRANCHES line verification, race condition documentation)
  • Fix fail: command not found in BATS tests — replace undefined fail() with error() (8 sites in concurrent-make-test.bats and make-test.bats)
  • Update CLAUDE.md with CI monitoring rules and multi-session PR guard
  • Add CI and Contributing section to README

Notes

This PR pairs with pgxntool PR #33 (add-ci branch). The CI in this repo resolves the pgxntool branch by matching the PR branch name — since both are now named add-ci, the full cross-repo workflow is exercised.

🤖 Generated with Claude Code

jnasbyupgrade and others added 15 commits May 15, 2026 14:33
Explains the cross-repo PR workflow, branch naming convention,
how the pgxntool CI wait logic works, the no-test-pr label and
its write-protection, and what branch protection enforces.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- .github/workflows/ci.yml: test against matching pgxntool branch
  (falls back to master if no matching branch exists)
- CLAUDE.md: warn before touching existing PRs not opened in this session

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
git subtree add refuses to work with shallow clones; fails with
"shallow roots are not allowed to be updated" which looks like a
remote/ref problem rather than a depth issue.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- gem install asciidoctor instead of apt ruby-asciidoctor: the apt
  package binary is not on PATH in pgxn-tools containers (diagnosed
  from base.mk: "Could not find asciidoc or asciidoctor")
- Print '=== BRANCHES: ===' line in resolve step so CI logs clearly
  show which pgxntool branch was selected for the run

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Shell-script-driven skill that monitors GitHub Actions runs across both
repos after a push. Key features:
- Uses --commit SHA when available to avoid branch-race-condition
- Fast log API path for BRANCHES line extraction (~1s vs 3-10s zip)
- Parallel monitoring of both repos by default
- Per-job pass/fail summary + failure log tail
- 10-min timeout for pgxntool (accounts for 5-min test-PR wait)
- Prefixes all output with [repo] for readability when interleaved

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The concurrent-make-test.bats test runs two `make test` processes
simultaneously to verify database-name isolation. Both processes check
$(datadir)/extension/pgtap.control and, finding it absent, both invoke
`pgxn install pgtap --sudo` concurrently. Their simultaneous `gmake
install` calls race to write files into the shared
/usr/share/postgresql/<pg>/extension/ directory, producing:

  install: cannot create regular file '.../pgtap--X.Y.Z.sql': File exists
  install: cannot change permissions of '...': No such file or directory

The race is most pronounced on older PostgreSQL versions (e.g. PG13)
where pgtap's build applies more compat patches, widening the window
during which both installs run in parallel.

Fix: add a Pre-install pgtap step before `make test`. With pgtap.control
already present, both concurrent make processes find the Make target's
prerequisite satisfied and skip the install entirely — no race occurs.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Script now prints OVERALL: ALL_PASS/FAIL/TIMEOUT as the last line
- Exit codes: 0=pass, 1=fail, 2=timeout (distinguishable from failure)
- SKILL.md documents the contract with a table so Claude knows exactly
  what to check without parsing the full output

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- monitor-ci.sh: fix sed delimiter collision when $label contains '/'
  (sed "s/^/$label /" broke because $label = "[repo/name]"; use '|' delimiter)
- monitor-ci.sh: initialize pid_test/pid_pgxn before case statement to
  prevent "unbound variable" error with set -u when repos=both
- SKILL.md: add race condition warning box; clarify BRANCHES line
  verification as primary safeguard against --branch picking wrong run
- CLAUDE.md: replace vague CI monitoring rule with specific SHA + BRANCHES
  line guidance

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
`fail` is not a BATS built-in and is not defined anywhere in the test
helpers. When a test assertion fails and `fail "message"` is called,
bash reports "fail: command not found" (exit 127), which masks the real
failure message entirely.

The correct replacement is `error`, which is defined in test/lib/helpers.bash:
  error() { out "ERROR: $*"; return 1; }

8 sites fixed across two files:
- test/standard/concurrent-make-test.bats (7 uses)
- test/standard/make-test.bats (1 use)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- README: rewrite CI and Contributing section to reflect new fast
  check-test-pr behavior, 'commit-with-no-tests' label, clear
  instructions for "no paired test PR" failures
- Remove references to 5-minute wait and old 'no-test-pr' label name

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add a prominent comment directing maintainers to keep this job in sync
with the copy in pgxntool/ci.yml (used in the commit-with-no-tests path),
and note the TODO to replace it with a reusable workflow.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ror handling

Reusable workflow:
- Extract test job to .github/workflows/run-tests.yml (workflow_call)
- Split old monolithic test job into resolve + test jobs; resolve runs
  once and outputs the pgxntool branch; test calls the reusable workflow
- Eliminates WARNING duplication comment — single source of truth now

Fork support:
- monitor-ci.sh: derive REPO_TEST/REPO_PGXN from 'gh repo view' instead
  of hardcoding Postgres-Extensions, so fork PRs monitor the right repos
- ci.yml resolve step: use ${{ github.repository_owner }} instead of
  hardcoded org for the pgxntool ls-remote and checkout

Monitor improvements:
- Bump timeouts: TIMEOUT_TEST 300→900s, TIMEOUT_PGXN 600→2100s
  (pgxntool CI now polls up to 20min waiting for test CI to complete)
- Update SKILL.md: cascading failure guidance clarified; 5min→20min
- Update README: fix "why two repos" explanation, clarify that pgxntool
  changes should almost always have paired test changes, explain the
  CI flow (pgxntool CI waits for test CI rather than running itself)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…rkflow call

Document the cross-repo reusable workflow tradeoffs, merge order constraint,
and @Branch@master transition requirement. Add a comment in ci.yml pointing
to CLAUDE.md so neither humans nor Claude miss the context.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 19, 2026

Review Change Stack

📝 Walkthrough

Walkthrough

This PR adds PR-triggered CI that coordinates tests between pgxntool-test and pgxntool, a reusable run-tests.yml workflow that runs make test across PostgreSQL versions in a container, a post-push CI monitoring skill and monitor-ci.sh script, small Bats test assertion changes (fail -> error), and supporting documentation and guidance updates.

Sequence Diagram(s)

sequenceDiagram
  participant PR as Pull Request
  participant Resolve as resolve Job
  participant PgxnRepo as pgxntool Repo
  participant TestJob as test Job
  participant Reusable as run-tests.yml

  PR->>Resolve: pull_request event
  Resolve->>PgxnRepo: git ls-remote (check branch)
  PgxnRepo-->>Resolve: branch exists or not
  Resolve->>TestJob: output pgxntool-ref
  TestJob->>Reusable: workflow_call with refs
  Reusable-->>TestJob: test results
Loading
sequenceDiagram
  participant User as CLI User
  participant Script as monitor-ci.sh
  participant GHCLI as gh CLI
  participant Actions as GitHub Actions API
  participant Logs as Job Logs

  User->>Script: invoke monitor for repo/branch/sha
  Script->>GHCLI: gh run list / gh run view
  GHCLI-->>Script: run ID(s)
  Script->>Actions: fetch job logs -> extract === BRANCHES:
  Script->>Actions: poll run status
  Actions-->>Script: status updates / conclusions
  Script->>Logs: fetch last lines for failed jobs
  Script-->>User: OVERALL: <STATUS>
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related issues

  • #15: Implements the cross-repo workflows and branch-matching orchestration described in that issue.

Poem

🐰 I hopped through branches, logs in paw,

Watched workflows run and test the draw,
A script to monitor, brave and spry,
Two repos watched beneath the sky,
I cheered aloud: "ALL_PASS!" with a sigh.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically summarizes the main changes: adding CI workflows, the /ci skill, and test fixes. It covers the primary deliverables without being vague or off-topic.
Description check ✅ Passed The description is directly related to the changeset, providing a detailed summary of CI workflows, /ci skill implementation, BATS test fixes, and documentation updates that match the file changes.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch add-ci

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.claude/skills/ci/scripts/monitor-ci.sh:
- Around line 188-193: The case arms for 'pgxntool-test' and 'pgxntool'
currently force any non-zero exit to 1 (using "|| exit_code=1"), which loses
distinct exit codes like timeout (2); change each arm to call monitor_one and
capture its exact exit status (e.g., run monitor_one "$REPO_TEST" "$BRANCH"
"$SHA_TEST" "$TIMEOUT_TEST" then set exit_code to its $? only if non-zero) so
monitor_one's return codes are preserved; reference the monitor_one invocation
in the pgxntool-test and pgxntool case branches and ensure you do not coerce
non-zero results to 1.

In @.claude/skills/ci/SKILL.md:
- Around line 54-65: The unlabeled fenced code block in
.claude/skills/ci/SKILL.md (the block starting at the shown sample output)
triggers markdownlint MD040; fix it by adding a language identifier (e.g.,
"text") after the opening triple backticks so the block becomes ```text,
ensuring the fenced output is labeled and the lint rule is satisfied.

In @.github/workflows/ci.yml:
- Around line 1-4: Add explicit least-privilege workflow token permissions by
inserting a top-level permissions block (e.g., under the same scope as "name:
CI" and "on: pull_request") and set permissions: contents: read so the workflow
only has read access to repository contents; ensure the new permissions block is
placed at the root of the workflow YAML (not nested under a job) so it applies
to the entire run.
- Around line 21-33: The script currently expands github.head_ref and
github.repository_owner directly into the run block causing shell-injection
risk; instead set them in the job's env (e.g., export PR_HEAD and REPO_OWNER env
vars) and reference them as safely quoted shell variables (PR_HEAD, REPO_OWNER)
in the run steps; validate PR_HEAD against a safe pattern (e.g., allow only
[A-Za-z0-9._/-] and non-empty) before using it to set BRANCH, and then use
BRANCH in the git ls-remote call and when writing to GITHUB_OUTPUT to avoid
untrusted template expansion and injection.

In @.github/workflows/run-tests.yml:
- Line 1: Add a top-level GitHub Actions permissions block limiting tokens to
read-only for repository contents (permissions: contents: read) and update both
uses of actions/checkout (the two checkout steps present in the workflow) to set
persist-credentials: false to avoid leaving the workflow token in the
checked-out repository; ensure you update each checkout invocation referenced in
the file so both occurrences use persist-credentials: false.
- Line 23: The workflow currently uses an unpinned container reference
("container: pgxn/pgxn-tools"); replace it with an image pinned to a digest (for
example "container: pgxn/pgxn-tools@sha256:<DIGEST>") to ensure reproducible CI
runs. Locate the "container: pgxn/pgxn-tools" entry in
.github/workflows/run-tests.yml, obtain the correct sha256 digest from the
registry for the desired tag/version, and update the line to include that
digest; verify the workflow still runs locally or in CI after the change.
- Around line 33-39: The echo in the run step interpolates caller-controlled
workflow inputs (inputs.pgxntool-branch and inputs.pgxntool-test-ref) directly
into the shell, creating a command-injection risk; define job-level environment
variables (e.g., PGXNTOOL_BRANCH and PGXNTOOL_TEST_REF) populated from the
workflow inputs, reference those env vars inside the run block using the safe
$PGXNTOOL_BRANCH and $PGXNTOOL_TEST_REF syntax, and update the echo line (and
any other run steps that use those inputs, such as the one at line ~85) to use
the env variables instead of direct GitHub Actions input interpolation.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 028b49c6-2545-4d47-8a90-57d07a142b8b

📥 Commits

Reviewing files that changed from the base of the PR and between 06aab6a and 90e2cad.

📒 Files selected for processing (9)
  • .claude/skills/ci/SKILL.md
  • .claude/skills/ci/scripts/monitor-ci.sh
  • .github/workflows/CLAUDE.md
  • .github/workflows/ci.yml
  • .github/workflows/run-tests.yml
  • CLAUDE.md
  • README.md
  • test/standard/concurrent-make-test.bats
  • test/standard/make-test.bats

Comment on lines +188 to +193
pgxntool-test)
monitor_one "$REPO_TEST" "$BRANCH" "$SHA_TEST" "$TIMEOUT_TEST" || exit_code=1
;;
pgxntool)
monitor_one "$REPO_PGXN" "$BRANCH" "$SHA_PGXN" "$TIMEOUT_PGXN" || exit_code=1
;;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Preserve monitor_one return codes in single-repo mode.

Line 189 and Line 192 force any non-zero result to 1, so timeout (2) is downgraded to fail and OVERALL: TIMEOUT cannot occur for single-repo runs.

Suggested patch
 case "$REPOS" in
   pgxntool-test)
-    monitor_one "$REPO_TEST" "$BRANCH" "$SHA_TEST" "$TIMEOUT_TEST" || exit_code=1
+    monitor_one "$REPO_TEST" "$BRANCH" "$SHA_TEST" "$TIMEOUT_TEST" \
+      || { r=$?; [[ $r -gt $exit_code ]] && exit_code=$r; }
     ;;
   pgxntool)
-    monitor_one "$REPO_PGXN" "$BRANCH" "$SHA_PGXN" "$TIMEOUT_PGXN" || exit_code=1
+    monitor_one "$REPO_PGXN" "$BRANCH" "$SHA_PGXN" "$TIMEOUT_PGXN" \
+      || { r=$?; [[ $r -gt $exit_code ]] && exit_code=$r; }
     ;;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
pgxntool-test)
monitor_one "$REPO_TEST" "$BRANCH" "$SHA_TEST" "$TIMEOUT_TEST" || exit_code=1
;;
pgxntool)
monitor_one "$REPO_PGXN" "$BRANCH" "$SHA_PGXN" "$TIMEOUT_PGXN" || exit_code=1
;;
pgxntool-test)
monitor_one "$REPO_TEST" "$BRANCH" "$SHA_TEST" "$TIMEOUT_TEST" \
|| { r=$?; [[ $r -gt $exit_code ]] && exit_code=$r; }
;;
pgxntool)
monitor_one "$REPO_PGXN" "$BRANCH" "$SHA_PGXN" "$TIMEOUT_PGXN" \
|| { r=$?; [[ $r -gt $exit_code ]] && exit_code=$r; }
;;
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.claude/skills/ci/scripts/monitor-ci.sh around lines 188 - 193, The case
arms for 'pgxntool-test' and 'pgxntool' currently force any non-zero exit to 1
(using "|| exit_code=1"), which loses distinct exit codes like timeout (2);
change each arm to call monitor_one and capture its exact exit status (e.g., run
monitor_one "$REPO_TEST" "$BRANCH" "$SHA_TEST" "$TIMEOUT_TEST" then set
exit_code to its $? only if non-zero) so monitor_one's return codes are
preserved; reference the monitor_one invocation in the pgxntool-test and
pgxntool case branches and ensure you do not coerce non-zero results to 1.

Comment on lines +54 to +65
```
[pgxntool-test] Run 12345678 found
[pgxntool-test] === BRANCHES: pgxntool-test=feature/foo pgxntool=feature/foo ===
[pgxntool-test] Polling... (running: 🐘 PostgreSQL 13, 🐘 PostgreSQL 15)
[pgxntool-test] PASS 🐘 PostgreSQL 12
[pgxntool-test] PASS 🐘 PostgreSQL 15
[pgxntool-test] FAIL 🐘 PostgreSQL 13
[pgxntool-test] Run completed: FAILURE
[pgxntool-test] === FAILURE: 🐘 PostgreSQL 13 ===
... failure log lines ...
OVERALL: FAIL
```
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Specify a language for the fenced output block.

Line 54 uses an unlabeled fenced block, which triggers markdownlint MD040.

Suggested patch
-```
+```text
 [pgxntool-test] Run 12345678 found
 [pgxntool-test] === BRANCHES: pgxntool-test=feature/foo pgxntool=feature/foo ===
 [pgxntool-test] Polling... (running: 🐘 PostgreSQL 13, 🐘 PostgreSQL 15)
 [pgxntool-test] PASS  🐘 PostgreSQL 12
 [pgxntool-test] PASS  🐘 PostgreSQL 15
 [pgxntool-test] FAIL  🐘 PostgreSQL 13
 [pgxntool-test] Run completed: FAILURE
 [pgxntool-test] === FAILURE: 🐘 PostgreSQL 13 ===
 ... failure log lines ...
 OVERALL: FAIL
</details>

<!-- suggestion_start -->

<details>
<summary>📝 Committable suggestion</summary>

> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

```suggestion

🧰 Tools
🪛 markdownlint-cli2 (0.22.1)

[warning] 54-54: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.claude/skills/ci/SKILL.md around lines 54 - 65, The unlabeled fenced code
block in .claude/skills/ci/SKILL.md (the block starting at the shown sample
output) triggers markdownlint MD040; fix it by adding a language identifier
(e.g., "text") after the opening triple backticks so the block becomes ```text,
ensuring the fenced output is labeled and the lint rule is satisfied.

Comment thread .github/workflows/ci.yml
Comment on lines +1 to +4
name: CI

on:
pull_request:
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

cat -n .github/workflows/ci.yml

Repository: Postgres-Extensions/pgxntool-test

Length of output: 2450


🏁 Script executed:

cat -n .github/workflows/run-tests.yml

Repository: Postgres-Extensions/pgxntool-test

Length of output: 7408


Add explicit workflow token permissions for least-privilege access.

This workflow relies on default token permissions and should declare explicit minimal permissions to prevent future config drift from widening access unnecessarily. The workflow only performs read operations (checking out code from public repositories), so permissions: contents: read is appropriate and sufficient.

Proposed change
 name: CI
+
+permissions:
+  contents: read

 on:
   pull_request:
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
name: CI
on:
pull_request:
name: CI
permissions:
contents: read
on:
pull_request:
🧰 Tools
🪛 zizmor (1.25.2)

[warning] 1-50: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block

(excessive-permissions)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/ci.yml around lines 1 - 4, Add explicit least-privilege
workflow token permissions by inserting a top-level permissions block (e.g.,
under the same scope as "name: CI" and "on: pull_request") and set permissions:
contents: read so the workflow only has read access to repository contents;
ensure the new permissions block is placed at the root of the workflow YAML (not
nested under a job) so it applies to the entire run.

Comment thread .github/workflows/ci.yml
Comment on lines +21 to +33
run: |
BRANCH="${{ github.head_ref }}"

# Check whether pgxntool has a branch with the same name as this PR.
# If it does, use it — changes in this test PR may depend on changes
# in a paired pgxntool branch. If not, fall back to master.
#
# --exit-code makes git ls-remote return non-zero when the ref is
# absent, so we can branch on the result cleanly without parsing output.
if git ls-remote --exit-code --heads \
https://github.com/${{ github.repository_owner }}/pgxntool.git \
"refs/heads/${BRANCH}" > /dev/null 2>&1; then
echo "ref=${BRANCH}" >> "$GITHUB_OUTPUT"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

fd -t f "ci.yml" | head -20

Repository: Postgres-Extensions/pgxntool-test

Length of output: 59


🏁 Script executed:

cat -n .github/workflows/ci.yml | head -50

Repository: Postgres-Extensions/pgxntool-test

Length of output: 2450


🏁 Script executed:

cat -n .github/workflows/ci.yml | sed -n '15,40p'

Repository: Postgres-Extensions/pgxntool-test

Length of output: 1380


🌐 Web query:

GitHub Actions github.head_ref security shell injection escaping context

💡 Result:

In GitHub Actions, github.head_ref should be treated as untrusted input, because it comes from the pull request’s source branch and can contain attacker-controlled characters/values [1]. If you interpolate it directly into a shell step (run:/script:) without proper quoting/escaping, it can lead to shell injection/code/script injection: expressions are evaluated by Actions and substituted verbatim into the temporary shell script that the runner executes [2]. Key points about “escaping” and which contexts are safe 1) github.head_ref itself - The official contexts reference describes github.head_ref as the head/source branch name for pull_request and pull_request_target events [1]. That value originates from the PR, so a malicious PR can choose the branch name. - GitHub’s security guidance warns to consider whether code might execute untrusted input and references script injections as a risk [2]. 2) Expression contexts (YAML conditions / if:) are not the same risk as run: shell - workflow conditions such as if: are evaluated by GitHub’s internal engine, not by a shell interpreting the result; this is why some linters flag github.head_ref usage generally but treat it differently in if contexts (as discussed in actionlint issue) [3]. - Practical takeaway: using github.head_ref inside if: predicates is generally not “shell injection” (it’s still untrusted logically—don’t use it to build commands later), but don’t assume it’s automatically safe everywhere. 3) The real injection risk is when the value flows into a shell script - GitHub Docs explain that run executes within a temporary shell script, and that expressions inside ${{ }} are evaluated and then substituted before the shell runs; this substitution can make the workflow vulnerable to shell command injection [2]. - In other words, there is no automatic shell escaping/neutralization for ${{ github.head_ref }} when it ends up inside a run script; treat it like raw text going into a shell program [2]. What to do instead (defensive patterns) 1) Prefer passing the value via env: and then using shell quoting - The general defensive guidance is: assign untrusted context values to an environment variable, then reference that variable using native shell quoting rules (so metacharacters are treated as string data, not interpreted commands) [2]. - Example remediation pattern (conceptual): - bad: run: echo ${{ github.head_ref }} (if unquoted, metacharacters could be interpreted) - better: env: HEAD_REF: ${{ github.head_ref }}; run: echo "$HEAD_REF" 2) If you must use it in a command-line argument, quote it for the shell - GitHub’s docs and multiple security analyses emphasize that injection occurs because the substituted value becomes part of the generated shell script [2]. Therefore, ensure the value is safely quoted in the run script (e.g., double quotes for bash, and avoid constructing unquoted/partially-quoted commands). 3) Avoid using github.head_ref to directly build “code execution” strings - Don’t build scripts, eval-like constructs, or pipelines by concatenating untrusted strings into run: - Instead, validate against an allowlist (e.g., branch names you expect) or map to safe derived values. 4) Be aware of broader workflow trust issues - Many real-world issues involve running workflows in pull_request_target (or other privileged contexts) where attacker-controlled values can reach sensitive sinks; GitHub’s security hardening docs emphasize least privilege and caution about risk when workflows/actions can be compromised [4]. Bottom line - github.head_ref is untrusted because it’s sourced from the pull request branch name [1]. - It becomes dangerous when injected into run/script where the expression is substituted before the shell executes, enabling shell injection/code injection if you don’t quote/sanitize correctly [2]. - Using github.head_ref in non-shell contexts like if: predicates is generally not “shell injection,” but still keep untrusted-flow discipline: only use it for logic checks, not for building shell commands [3].

Citations:


Move untrusted template expansion to environment variables and validate branch name.

github.head_ref is PR-controlled input. When directly expanded in run: via ${{ }}, it's substituted verbatim into the shell script before execution, creating a shell injection risk. GitHub's security guidance recommends passing such values through env: instead, where they're accessed as variables with proper shell quoting.

The current code also directly expands ${{ github.repository_owner }} in the git URL. Both should be moved to the env: block.

Proposed hardening
       - name: Resolve pgxntool branch
         id: pgxntool-ref
+        env:
+          BRANCH: ${{ github.head_ref }}
+          REPO_OWNER: ${{ github.repository_owner }}
         run: |
-          BRANCH="${{ github.head_ref }}"
+          git check-ref-format --branch "$BRANCH" >/dev/null 2>&1 || {
+            echo "Invalid branch name: $BRANCH" >&2
+            exit 1
+          }

           # Check whether pgxntool has a branch with the same name as this PR.
@@
           if git ls-remote --exit-code --heads \
-               https://github.com/${{ github.repository_owner }}/pgxntool.git \
+               "https://github.com/${REPO_OWNER}/pgxntool.git" \
                "refs/heads/${BRANCH}" > /dev/null 2>&1; then
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
run: |
BRANCH="${{ github.head_ref }}"
# Check whether pgxntool has a branch with the same name as this PR.
# If it does, use it — changes in this test PR may depend on changes
# in a paired pgxntool branch. If not, fall back to master.
#
# --exit-code makes git ls-remote return non-zero when the ref is
# absent, so we can branch on the result cleanly without parsing output.
if git ls-remote --exit-code --heads \
https://github.com/${{ github.repository_owner }}/pgxntool.git \
"refs/heads/${BRANCH}" > /dev/null 2>&1; then
echo "ref=${BRANCH}" >> "$GITHUB_OUTPUT"
env:
BRANCH: ${{ github.head_ref }}
REPO_OWNER: ${{ github.repository_owner }}
run: |
git check-ref-format --branch "$BRANCH" >/dev/null 2>&1 || {
echo "Invalid branch name: $BRANCH" >&2
exit 1
}
# Check whether pgxntool has a branch with the same name as this PR.
# If it does, use it — changes in this test PR may depend on changes
# in a paired pgxntool branch. If not, fall back to master.
#
# --exit-code makes git ls-remote return non-zero when the ref is
# absent, so we can branch on the result cleanly without parsing output.
if git ls-remote --exit-code --heads \
"https://github.com/${REPO_OWNER}/pgxntool.git" \
"refs/heads/${BRANCH}" > /dev/null 2>&1; then
echo "ref=${BRANCH}" >> "$GITHUB_OUTPUT"
🧰 Tools
🪛 actionlint (1.7.12)

[error] 21-21: "github.head_ref" is potentially untrusted. avoid using it directly in inline scripts. instead, pass it through an environment variable. see https://docs.github.com/en/actions/reference/security/secure-use#good-practices-for-mitigating-script-injection-attacks for more details

(expression)

🪛 zizmor (1.25.2)

[error] 22-22: code injection via template expansion (template-injection): may expand into attacker-controllable code

(template-injection)


[warning] 31-31: code injection via template expansion (template-injection): may expand into attacker-controllable code

(template-injection)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/ci.yml around lines 21 - 33, The script currently expands
github.head_ref and github.repository_owner directly into the run block causing
shell-injection risk; instead set them in the job's env (e.g., export PR_HEAD
and REPO_OWNER env vars) and reference them as safely quoted shell variables
(PR_HEAD, REPO_OWNER) in the run steps; validate PR_HEAD against a safe pattern
(e.g., allow only [A-Za-z0-9._/-] and non-empty) before using it to set BRANCH,
and then use BRANCH in the git ls-remote call and when writing to GITHUB_OUTPUT
to avoid untrusted template expansion and injection.

@@ -0,0 +1,138 @@
name: Run Tests
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

cat -n .github/workflows/run-tests.yml

Repository: Postgres-Extensions/pgxntool-test

Length of output: 7408


Harden workflow token scope and checkout credential handling.

This reusable workflow lacks explicit permissions and both checkouts persist credentials by default, unnecessarily increasing token exposure when called from PR CI contexts. Add a permissions block restricting to contents: read and set persist-credentials: false on both checkout steps (lines 40-45 and 53-58).

Proposed hardening
 name: Run Tests
+
+permissions:
+  contents: read
@@
       - name: Check out pgxntool-test
         uses: actions/checkout@v4
         with:
+          persist-credentials: false
           repository: ${{ github.repository_owner }}/pgxntool-test
@@
       - name: Check out pgxntool
         uses: actions/checkout@v4
         with:
+          persist-credentials: false
           repository: ${{ github.repository_owner }}/pgxntool
🧰 Tools
🪛 zizmor (1.25.2)

[warning] 1-139: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block

(excessive-permissions)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/run-tests.yml at line 1, Add a top-level GitHub Actions
permissions block limiting tokens to read-only for repository contents
(permissions: contents: read) and update both uses of actions/checkout (the two
checkout steps present in the workflow) to set persist-credentials: false to
avoid leaving the workflow token in the checked-out repository; ensure you
update each checkout invocation referenced in the file so both occurrences use
persist-credentials: false.

test:
name: 🐘 PostgreSQL ${{ matrix.pg }}
runs-on: ubuntu-latest
container: pgxn/pgxn-tools
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

# First, let's examine the workflow file to confirm the current state
cat -n .github/workflows/run-tests.yml | head -40

Repository: Postgres-Extensions/pgxntool-test

Length of output: 1635


🏁 Script executed:

# Check if there are other container references in workflows
rg "container:" .github/workflows/ -A 2

Repository: Postgres-Extensions/pgxntool-test

Length of output: 232


🏁 Script executed:

# Check the entire workflow file structure to understand the context better
wc -l .github/workflows/run-tests.yml

Repository: Postgres-Extensions/pgxntool-test

Length of output: 113


🏁 Script executed:

# Check if there's any documentation or configuration about container versions
rg -i "pgxn-tools|container|image" .github/ --type yaml --type md -B 2 -A 2

Repository: Postgres-Extensions/pgxntool-test

Length of output: 1785


🏁 Script executed:

# Check git history or any comments about this image
git log --all --oneline --grep="container\|pgxn-tools\|image" 2>/dev/null | head -20

Repository: Postgres-Extensions/pgxntool-test

Length of output: 59


🏁 Script executed:

# Look for any build or deployment documentation
find . -type f \( -name "*.md" -o -name "*.txt" -o -name "README*" \) -exec grep -l "pgxn-tools\|container" {} \; 2>/dev/null

Repository: Postgres-Extensions/pgxntool-test

Length of output: 59


🏁 Script executed:

# Check for any other container or image references in other workflows
find .github/workflows -name "*.yml" -o -name "*.yaml" | xargs grep -l "container:" 2>/dev/null

Repository: Postgres-Extensions/pgxntool-test

Length of output: 109


🏁 Script executed:

# Get complete context around line 23 to understand job structure
sed -n '20,30p' .github/workflows/run-tests.yml

Repository: Postgres-Extensions/pgxntool-test

Length of output: 348


Pin the CI container image to a digest.

Floating image references make builds non-reproducible and increase supply-chain risk. Update line 23 to pin to a specific digest:

-    container: pgxn/pgxn-tools
+    container: pgxn/pgxn-tools@sha256:<trusted-digest>
🧰 Tools
🪛 zizmor (1.25.2)

[error] 23-23: unpinned image references (unpinned-images): container image is unpinned

(unpinned-images)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/run-tests.yml at line 23, The workflow currently uses an
unpinned container reference ("container: pgxn/pgxn-tools"); replace it with an
image pinned to a digest (for example "container:
pgxn/pgxn-tools@sha256:<DIGEST>") to ensure reproducible CI runs. Locate the
"container: pgxn/pgxn-tools" entry in .github/workflows/run-tests.yml, obtain
the correct sha256 digest from the registry for the desired tag/version, and
update the line to include that digest; verify the workflow still runs locally
or in CI after the change.

Comment on lines +33 to +39
run: |
# Print both repos and exact refs at the top of every job so failures
# are easy to correlate to the right code. This is especially useful
# when debugging cross-repo issues where the wrong branch was picked.
# The === BRANCHES: === line is parsed by the CI monitor script.
echo "=== BRANCHES: pgxntool=${{ inputs.pgxntool-branch }} pgxntool-test=${{ inputs.pgxntool-test-ref }} ==="

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

cat -n .github/workflows/run-tests.yml

Repository: Postgres-Extensions/pgxntool-test

Length of output: 7408


Use environment variables for workflow inputs in shell commands.

inputs.pgxntool-branch and inputs.pgxntool-test-ref are caller-controlled. Interpolating them directly inside run: commands (lines 38, 85) exposes command injection paths. Move them to job-level environment variables and use $VAR syntax instead.

Proposed hardening
 jobs:
   test:
+    env:
+      PGXNTOOL_BRANCH: ${{ inputs.pgxntool-branch }}
+      PGXNTOOL_TEST_REF: ${{ inputs.pgxntool-test-ref }}
     steps:
       - name: Report CI context
         run: |
-          echo "=== BRANCHES: pgxntool=${{ inputs.pgxntool-branch }} pgxntool-test=${{ inputs.pgxntool-test-ref }} ==="
+          echo "=== BRANCHES: pgxntool=${PGXNTOOL_BRANCH} pgxntool-test=${PGXNTOOL_TEST_REF} ==="
       - name: Configure git for CI
         run: |
           cd pgxntool
-          git checkout -B "${{ inputs.pgxntool-branch }}"
+          git check-ref-format --branch "$PGXNTOOL_BRANCH" >/dev/null 2>&1
+          git checkout -B "$PGXNTOOL_BRANCH"
🧰 Tools
🪛 zizmor (1.25.2)

[error] 38-38: code injection via template expansion (template-injection): may expand into attacker-controllable code

(template-injection)


[error] 38-38: code injection via template expansion (template-injection): may expand into attacker-controllable code

(template-injection)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/run-tests.yml around lines 33 - 39, The echo in the run
step interpolates caller-controlled workflow inputs (inputs.pgxntool-branch and
inputs.pgxntool-test-ref) directly into the shell, creating a command-injection
risk; define job-level environment variables (e.g., PGXNTOOL_BRANCH and
PGXNTOOL_TEST_REF) populated from the workflow inputs, reference those env vars
inside the run block using the safe $PGXNTOOL_BRANCH and $PGXNTOOL_TEST_REF
syntax, and update the echo line (and any other run steps that use those inputs,
such as the one at line ~85) to use the env variables instead of direct GitHub
Actions input interpolation.

When a SHA is provided, GitHub may not index the new run immediately.
Previously the script fell back to --branch on the first attempt, picking
up the previous run. Now it retries the --commit SHA lookup for up to 30s
before allowing the branch fallback.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.claude/skills/ci/scripts/monitor-ci.sh:
- Around line 13-17: The header of monitor-ci.sh documents exit code 3 (NO_RUNS)
but the script returns 1 in the "no CI run found" branch; either remove exit
code 3 from the comment block or implement it: change the branch that currently
returns 1 for "no runs found" to exit 3 (NO_RUNS) and update the final status
reporting logic (the block that handles exit codes near the end of the script)
to include a case for exit code 3 with an appropriate message; locate the "no CI
run found" return in the polling/wait loop and the final status switch/if that
reports statuses and update both so comments and behavior match.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 2865c3ad-fcec-4bb7-880a-de31ad736903

📥 Commits

Reviewing files that changed from the base of the PR and between 90e2cad and 1b0ed7d.

📒 Files selected for processing (1)
  • .claude/skills/ci/scripts/monitor-ci.sh

Comment on lines +13 to +17
# Exit codes:
# 0 : ALL_PASS — all jobs succeeded
# 1 : FAIL — one or more jobs failed
# 2 : TIMEOUT — run(s) did not complete within the timeout
# 3 : NO_RUNS — no CI run found for this branch after waiting
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Exit code 3 (NO_RUNS) is documented but never returned.

The header documents exit code 3 for "no CI run found," but line 80 returns 1 in that case. Either update the documentation to remove exit code 3, or update line 80 to return 3.

Option A: Update documentation to match implementation
 # Exit codes:
 #   0 : ALL_PASS  — all jobs succeeded
-#   1 : FAIL      — one or more jobs failed
+#   1 : FAIL      — one or more jobs failed, or no CI run found
 #   2 : TIMEOUT   — run(s) did not complete within the timeout
-#   3 : NO_RUNS   — no CI run found for this branch after waiting
Option B: Update implementation to match documentation
       if [[ $elapsed -ge $timeout ]]; then
         echo "$label ERROR: no CI run found after ${timeout}s" >&2
-        return 1
+        return 3
       fi

Also update the final status reporting (lines 213-218) to handle exit code 3:

 if [[ $exit_code -eq 0 ]]; then
   echo "OVERALL: ALL_PASS"
 elif [[ $exit_code -eq 2 ]]; then
   echo "OVERALL: TIMEOUT"
+elif [[ $exit_code -eq 3 ]]; then
+  echo "OVERALL: NO_RUNS"
 else
   echo "OVERALL: FAIL"
 fi
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# Exit codes:
# 0 : ALL_PASS — all jobs succeeded
# 1 : FAIL — one or more jobs failed
# 2 : TIMEOUT — run(s) did not complete within the timeout
# 3 : NO_RUNS — no CI run found for this branch after waiting
# Exit codes:
# 0 : ALL_PASS — all jobs succeeded
# 1 : FAIL — one or more jobs failed, or no CI run found
# 2 : TIMEOUT — run(s) did not complete within the timeout
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.claude/skills/ci/scripts/monitor-ci.sh around lines 13 - 17, The header of
monitor-ci.sh documents exit code 3 (NO_RUNS) but the script returns 1 in the
"no CI run found" branch; either remove exit code 3 from the comment block or
implement it: change the branch that currently returns 1 for "no runs found" to
exit 3 (NO_RUNS) and update the final status reporting logic (the block that
handles exit codes near the end of the script) to include a case for exit code 3
with an appropriate message; locate the "no CI run found" return in the
polling/wait loop and the final status switch/if that reports statuses and
update both so comments and behavior match.

The two inputs were always passed the same value by every caller. The
distinction (ref could be a SHA while branch must be a name) was
forward-looking but never used. Collapsing removes confusion about why
two inputs existed and simplifies the interface.

run-tests.yml: remove pgxntool-ref input; use pgxntool-branch as the
checkout ref directly (branch names are valid checkout refs).
ci.yml: drop the now-redundant pgxntool-ref: line from the with: block.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (5)
.github/workflows/run-tests.yml (3)

19-19: ⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Pin third-party container/action references to immutable digests/SHAs.

Line 19 uses a floating container tag, and Line 37/Line 50 use mutable action tags. Pinning improves reproducibility and supply-chain integrity.

Also applies to: 37-37, 50-50

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/run-tests.yml at line 19, The workflow uses mutable image
and action tags; replace the floating container reference "container:
pgxn/pgxn-tools" with an immutable digest-pinned reference (e.g., the
image@sha256:... form) and update the GitHub action usages referenced at lines
37 and 50 to use commit SHA pins (owner/repo@sha256-or-commitSHA) instead of
mutable tags; locate the container declaration and the action steps (the step
entries that currently reference the mutable action tags) and swap them for
their corresponding digest/SHA-pinned versions to ensure reproducible builds.

1-2: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Restrict workflow token scope and disable checkout credential persistence.

Line 1 lacks explicit token permissions, and Line 37/Line 50 use default credential persistence. Tighten both.

Suggested patch
 name: Run Tests
+
+permissions:
+  contents: read
@@
       - name: Check out pgxntool-test
         uses: actions/checkout@v4
         with:
+          persist-credentials: false
           repository: ${{ github.repository_owner }}/pgxntool-test
@@
       - name: Check out pgxntool
         uses: actions/checkout@v4
         with:
+          persist-credentials: false
           repository: ${{ github.repository_owner }}/pgxntool

Also applies to: 36-55

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/run-tests.yml around lines 1 - 2, Update the workflow
header and checkout steps to restrict GITHUB_TOKEN scope and disable credential
persistence: add a top-level permissions map (e.g., permissions: contents: read)
alongside the existing name: Run Tests and ensure every actions/checkout step
uses persist-credentials: false (look for actions/checkout usage in the file).
If any job needs other token scopes, add only those specific permissions at the
job level rather than broad defaults.

28-35: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Use env vars for workflow inputs in shell and validate branch before checkout.

Line 34 and Line 81 currently template-expand caller-controlled inputs inside run: scripts. Pass inputs via job env, quote variables, and validate with git check-ref-format --branch.

Suggested patch
 jobs:
   test:
     name: 🐘 PostgreSQL ${{ matrix.pg }}
     runs-on: ubuntu-latest
     container: pgxn/pgxn-tools
+    env:
+      PGXNTOOL_BRANCH: ${{ inputs.pgxntool-branch }}
+      PGXNTOOL_TEST_REF: ${{ inputs.pgxntool-test-ref }}
@@
       - name: Report CI context
         run: |
@@
-          echo "=== BRANCHES: pgxntool=${{ inputs.pgxntool-branch }} pgxntool-test=${{ inputs.pgxntool-test-ref }} ==="
+          echo "=== BRANCHES: pgxntool=${PGXNTOOL_BRANCH} pgxntool-test=${PGXNTOOL_TEST_REF} ==="
@@
           cd pgxntool
-          git checkout -B "${{ inputs.pgxntool-branch }}"
+          git check-ref-format --branch "$PGXNTOOL_BRANCH" >/dev/null 2>&1 || {
+            echo "Invalid branch name: $PGXNTOOL_BRANCH" >&2
+            exit 1
+          }
+          git checkout -B "$PGXNTOOL_BRANCH"
@@
       - name: Run tests
         working-directory: pgxntool-test
         env:
@@
-          PGXNBRANCH: ${{ inputs.pgxntool-branch }}
+          PGXNBRANCH: ${{ env.PGXNTOOL_BRANCH }}

Also applies to: 62-82

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/run-tests.yml around lines 28 - 35, The workflow is
directly expanding caller-controlled inputs inside run: scripts (e.g. the Report
CI context step echo using ${{ inputs.pgxntool-branch }} and the checkout steps
later), which is unsafe; change the job to export those inputs into environment
variables (e.g. PGXNTOOL_BRANCH, PGXNTOOL_TEST_REF) and update the run blocks to
reference the quoted shell variables ("$PGXNTOOL_BRANCH"), and before any git
checkout or use of the branch variables run a validation step using git
check-ref-format --branch "$PGXNTOOL_BRANCH" (and for PGXNTOOL_TEST_REF if it's
a branch), failing fast on invalid refs; update the Report CI context echo and
all other run blocks that previously used templated inputs to use the env vars
instead and ensure all variable expansions are quoted.
.github/workflows/ci.yml (2)

21-33: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Remove direct template expansion of untrusted PR data in run:.

Line 22 and Line 31 interpolate context values directly into the shell script. Move them to env: and validate the branch name before using it.

Suggested patch
       - name: Resolve pgxntool branch
         id: pgxntool-ref
+        env:
+          BRANCH: ${{ github.head_ref }}
+          REPO_OWNER: ${{ github.repository_owner }}
         run: |
-          BRANCH="${{ github.head_ref }}"
+          git check-ref-format --branch "$BRANCH" >/dev/null 2>&1 || {
+            echo "Invalid branch name: $BRANCH" >&2
+            exit 1
+          }
 
@@
           if git ls-remote --exit-code --heads \
-               https://github.com/${{ github.repository_owner }}/pgxntool.git \
+               "https://github.com/${REPO_OWNER}/pgxntool.git" \
                "refs/heads/${BRANCH}" > /dev/null 2>&1; then
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/ci.yml around lines 21 - 33, The workflow exposes
untrusted PR data by interpolating ${{ github.head_ref }} and ${{
github.repository_owner }} directly into the run: script; move those template
values into env variables (e.g., BRANCH and REPO_OWNER) and reference the env
vars inside the script, then validate BRANCH (allow only a safe whitelist of
characters like letters, digits, ., _, -, /) before using it in the git
ls-remote invocation; keep using the existing BRANCH variable name and the git
ls-remote call and still write "ref=${BRANCH}" to GITHUB_OUTPUT only after the
branch name passes validation to prevent command injection or malformed refs.

1-4: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Add explicit root permissions to enforce least privilege.

Line 1 currently relies on default GITHUB_TOKEN scopes. Define minimal read-only permissions at the workflow root to prevent privilege drift.

Suggested patch
 name: CI
+
+permissions:
+  contents: read
 
 on:
   pull_request:
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/ci.yml around lines 1 - 4, Add an explicit root-level
permissions block to the "name: CI" workflow to limit GITHUB_TOKEN scopes;
update the top-level of the workflow to include a minimal read-only permissions
map (for example set permissions: contents: read, pull-requests: read, actions:
read) so the workflow no longer relies on default token scopes and enforces
least privilege for GITHUB_TOKEN.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/ci.yml:
- Around line 3-11: Add a top-level GitHub Actions concurrency block to the
workflow so superseded PR runs are cancelled: insert a concurrency section at
the top-level (above "jobs") that defines a group key using the repository and
pull request (e.g., using "${{ github.workflow }}-${{ github.ref }}-${{
github.head_ref }}" or similar) and set cancel-in-progress: true; this ensures
runs triggered by the "pull_request" event are deduplicated and in-progress
matrix runs are cancelled when new commits are pushed.

---

Duplicate comments:
In @.github/workflows/ci.yml:
- Around line 21-33: The workflow exposes untrusted PR data by interpolating ${{
github.head_ref }} and ${{ github.repository_owner }} directly into the run:
script; move those template values into env variables (e.g., BRANCH and
REPO_OWNER) and reference the env vars inside the script, then validate BRANCH
(allow only a safe whitelist of characters like letters, digits, ., _, -, /)
before using it in the git ls-remote invocation; keep using the existing BRANCH
variable name and the git ls-remote call and still write "ref=${BRANCH}" to
GITHUB_OUTPUT only after the branch name passes validation to prevent command
injection or malformed refs.
- Around line 1-4: Add an explicit root-level permissions block to the "name:
CI" workflow to limit GITHUB_TOKEN scopes; update the top-level of the workflow
to include a minimal read-only permissions map (for example set permissions:
contents: read, pull-requests: read, actions: read) so the workflow no longer
relies on default token scopes and enforces least privilege for GITHUB_TOKEN.

In @.github/workflows/run-tests.yml:
- Line 19: The workflow uses mutable image and action tags; replace the floating
container reference "container: pgxn/pgxn-tools" with an immutable digest-pinned
reference (e.g., the image@sha256:... form) and update the GitHub action usages
referenced at lines 37 and 50 to use commit SHA pins
(owner/repo@sha256-or-commitSHA) instead of mutable tags; locate the container
declaration and the action steps (the step entries that currently reference the
mutable action tags) and swap them for their corresponding digest/SHA-pinned
versions to ensure reproducible builds.
- Around line 1-2: Update the workflow header and checkout steps to restrict
GITHUB_TOKEN scope and disable credential persistence: add a top-level
permissions map (e.g., permissions: contents: read) alongside the existing name:
Run Tests and ensure every actions/checkout step uses persist-credentials: false
(look for actions/checkout usage in the file). If any job needs other token
scopes, add only those specific permissions at the job level rather than broad
defaults.
- Around line 28-35: The workflow is directly expanding caller-controlled inputs
inside run: scripts (e.g. the Report CI context step echo using ${{
inputs.pgxntool-branch }} and the checkout steps later), which is unsafe; change
the job to export those inputs into environment variables (e.g. PGXNTOOL_BRANCH,
PGXNTOOL_TEST_REF) and update the run blocks to reference the quoted shell
variables ("$PGXNTOOL_BRANCH"), and before any git checkout or use of the branch
variables run a validation step using git check-ref-format --branch
"$PGXNTOOL_BRANCH" (and for PGXNTOOL_TEST_REF if it's a branch), failing fast on
invalid refs; update the Report CI context echo and all other run blocks that
previously used templated inputs to use the env vars instead and ensure all
variable expansions are quoted.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 03178a09-996f-4e4c-936a-d393c4c96dad

📥 Commits

Reviewing files that changed from the base of the PR and between 1b0ed7d and b518ed9.

📒 Files selected for processing (2)
  • .github/workflows/ci.yml
  • .github/workflows/run-tests.yml

Comment thread .github/workflows/ci.yml
Comment on lines +3 to +11
on:
pull_request:
# We use 'pull_request' (not 'pull_request_target') deliberately.
# 'pull_request_target' runs with write access to the base repo, which is
# a security risk for untrusted fork code. Since this workflow only reads
# from other public repos (no secrets needed), 'pull_request' is correct
# and safe even for fork PRs.

jobs:
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

Add workflow concurrency to cancel superseded PR runs.

Without a concurrency group, rapid pushes create overlapping matrix runs and stale results. Add top-level concurrency with cancel-in-progress.

Suggested patch
 on:
   pull_request:
@@
+concurrency:
+  group: ci-pr-${{ github.event.pull_request.number || github.ref }}
+  cancel-in-progress: true
+
 jobs:
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
on:
pull_request:
# We use 'pull_request' (not 'pull_request_target') deliberately.
# 'pull_request_target' runs with write access to the base repo, which is
# a security risk for untrusted fork code. Since this workflow only reads
# from other public repos (no secrets needed), 'pull_request' is correct
# and safe even for fork PRs.
jobs:
on:
pull_request:
# We use 'pull_request' (not 'pull_request_target') deliberately.
# 'pull_request_target' runs with write access to the base repo, which is
# a security risk for untrusted fork code. Since this workflow only reads
# from other public repos (no secrets needed), 'pull_request' is correct
# and safe even for fork PRs.
concurrency:
group: ci-pr-${{ github.event.pull_request.number || github.ref }}
cancel-in-progress: true
jobs:
🧰 Tools
🪛 zizmor (1.25.2)

[warning] 3-9: insufficient job-level concurrency limits (concurrency-limits): workflow is missing concurrency setting

(concurrency-limits)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/ci.yml around lines 3 - 11, Add a top-level GitHub Actions
concurrency block to the workflow so superseded PR runs are cancelled: insert a
concurrency section at the top-level (above "jobs") that defines a group key
using the repository and pull request (e.g., using "${{ github.workflow }}-${{
github.ref }}-${{ github.head_ref }}" or similar) and set cancel-in-progress:
true; this ensures runs triggered by the "pull_request" event are deduplicated
and in-progress matrix runs are cancelled when new commits are pushed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant