Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions .github/workflows/create-tag.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ jobs:
runs-on: ubuntu-latest
permissions:
contents: write
actions: write

steps:
- name: Checkout
Expand Down Expand Up @@ -45,6 +46,15 @@ jobs:
git tag -a ${{ steps.version.outputs.tag }} -m "Release ${{ steps.version.outputs.tag }}"
git push origin ${{ steps.version.outputs.tag }}

- name: Dispatch publish workflow
if: steps.check-tag.outputs.exists == 'false'
env:
GH_TOKEN: ${{ github.token }}
run: |
gh workflow run publish-pypi-approval.yml \
--ref develop \
-f version=${{ steps.version.outputs.version }}
Comment on lines +49 to +56
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

Let reruns re-dispatch the publish workflow.

Line 50 keys this step off the tag being absent before the job ran. If Line 47 pushes the tag and Line 54 fails, a rerun will see exists=true and skip the dispatch entirely, so this workflow cannot recover from a transient handoff failure without a separate manual trigger.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/create-tag.yml around lines 49 - 56, The dispatch step
"Dispatch publish workflow" is currently gated by if:
steps.check-tag.outputs.exists == 'false' which prevents reruns from
re-dispatching after a transient failure; update that conditional to allow
reruns to proceed (for example change it to if: steps.check-tag.outputs.exists
== 'false' || github.run_attempt > 1) so a retried workflow can still run the gh
workflow run command when the original run failed; keep the gh workflow run
block and env GH_TOKEN unchanged.


- name: Tag already exists
if: steps.check-tag.outputs.exists == 'true'
run: |
Expand Down
192 changes: 137 additions & 55 deletions .github/workflows/publish-pypi-approval.yml
Original file line number Diff line number Diff line change
@@ -1,92 +1,174 @@
name: Publish to PyPI (with Approval)

on:
workflow_run:
workflows: ["Build and Test Distribution"]
types:
- completed
workflow_dispatch:
inputs:
version:
description: 'Version to publish (format: X.Y.Z, no "v" prefix)'
required: true
type: string

jobs:
publish-pypi:
if: github.event.workflow_run.conclusion == 'success'
build-wheel:
runs-on: ubuntu-latest
environment:
name: pypi-production
url: https://pypi.org/project/nemoguardrails/
permissions:
contents: write
id-token: write

env:
POETRY_VERSION: "1.8.2"
PYTHON_VERSION: "3.11"
steps:
- name: Detect version tag from workflow event
id: version
- name: Checkout tag
uses: actions/checkout@v4
with:
ref: v${{ inputs.version }}
fetch-depth: 0

- name: Validate version in pyproject.toml
run: |
HEAD_BRANCH="${{ github.event.workflow_run.head_branch }}"
VERSION_IN_FILE=$(grep '^version = ' pyproject.toml | sed 's/version = "\(.*\)"/\1/')
if [ "$VERSION_IN_FILE" != "${{ inputs.version }}" ]; then
echo "Version mismatch: pyproject.toml=$VERSION_IN_FILE, expected=${{ inputs.version }}"
exit 1
fi
Comment on lines 25 to +30
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Shell injection risk from inputs.version

${{ inputs.version }} is interpolated directly into the shell script at the expression evaluation layer, before the shell ever runs. A malicious (or accidentally malformed) value such as 1.0.0" ]; then echo pwned; exit 0; # would break out of the double-quoted comparison and execute arbitrary shell code. The same pattern repeats at line 171 (inside the awk command).

GitHub's own hardening guide recommends passing workflow inputs through environment variables so the shell sees them as data, never as syntax:

Suggested change
run: |
HEAD_BRANCH="${{ github.event.workflow_run.head_branch }}"
VERSION_IN_FILE=$(grep '^version = ' pyproject.toml | sed 's/version = "\(.*\)"/\1/')
if [ "$VERSION_IN_FILE" != "${{ inputs.version }}" ]; then
echo "Version mismatch: pyproject.toml=$VERSION_IN_FILE, expected=${{ inputs.version }}"
exit 1
fi
- name: Validate version in pyproject.toml
env:
INPUT_VERSION: ${{ inputs.version }}
run: |
VERSION_IN_FILE=$(grep '^version = ' pyproject.toml | sed 's/version = "\(.*\)"/\1/')
if [ "$VERSION_IN_FILE" != "$INPUT_VERSION" ]; then
echo "Version mismatch: pyproject.toml=$VERSION_IN_FILE, expected=$INPUT_VERSION"
exit 1
fi

Apply the same pattern to the awk -v version= call further down (line 171):

awk -v version="$INPUT_VERSION" '...' CHANGELOG.md
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/publish-pypi-approval.yml
Line: 25-30

Comment:
**Shell injection risk from `inputs.version`**

`${{ inputs.version }}` is interpolated directly into the shell script at the expression evaluation layer, before the shell ever runs. A malicious (or accidentally malformed) value such as `1.0.0" ]; then echo pwned; exit 0; #` would break out of the double-quoted comparison and execute arbitrary shell code. The same pattern repeats at line 171 (inside the `awk` command).

GitHub's own hardening guide recommends passing workflow inputs through environment variables so the shell sees them as data, never as syntax:

```suggestion
      - name: Validate version in pyproject.toml
        env:
          INPUT_VERSION: ${{ inputs.version }}
        run: |
          VERSION_IN_FILE=$(grep '^version = ' pyproject.toml | sed 's/version = "\(.*\)"/\1/')
          if [ "$VERSION_IN_FILE" != "$INPUT_VERSION" ]; then
            echo "Version mismatch: pyproject.toml=$VERSION_IN_FILE, expected=$INPUT_VERSION"
            exit 1
          fi
```

Apply the same pattern to the `awk -v version=` call further down (line 171):
```
awk -v version="$INPUT_VERSION" '...' CHANGELOG.md
```

How can I resolve this? If you propose a fix, please make it concise.


echo "Workflow triggered by: $HEAD_BRANCH"
- name: Set up Python
uses: actions/setup-python@v5
with:
python-version: ${{ env.PYTHON_VERSION }}

if [[ "$HEAD_BRANCH" =~ ^v[0-9]+\.[0-9]+\.[0-9]+$ ]]; then
TAG_NAME="$HEAD_BRANCH"
VERSION="${TAG_NAME#v}"
- name: Bootstrap poetry
run: |
curl -sSL https://install.python-poetry.org | POETRY_VERSION=${{ env.POETRY_VERSION }} python -

echo "version=${VERSION}" >> $GITHUB_OUTPUT
echo "tag=${TAG_NAME}" >> $GITHUB_OUTPUT
echo "artifact_name=${TAG_NAME}-build" >> $GITHUB_OUTPUT
- name: Update PATH
run: echo "$HOME/.local/bin" >> $GITHUB_PATH

echo "✅ Detected version tag: $TAG_NAME"
echo " Version: $VERSION"
echo " Artifact: ${TAG_NAME}-build"
else
echo "❌ Not triggered by a version tag: $HEAD_BRANCH"
echo "This workflow should only run for version tags (vX.Y.Z)"
exit 1
fi
- name: Configure poetry
run: poetry config virtualenvs.in-project true

- name: Checkout repository
uses: actions/checkout@v4
- name: Set up cache
uses: actions/cache@v4
id: cache
with:
ref: ${{ steps.version.outputs.tag }}
fetch-depth: 0
path: .venv
key: venv-${{ runner.os }}-${{ env.PYTHON_VERSION }}-${{ hashFiles('**/poetry.lock') }}

- name: Ensure cache is healthy
if: steps.cache.outputs.cache-hit == 'true'
run: timeout 10s poetry run pip --version || rm -rf .venv

- name: Make build script executable
run: chmod +x ./.github/scripts/build.sh

- name: Build distributions
run: ./.github/scripts/build.sh

- name: Upload build artifacts
uses: actions/upload-artifact@v4
with:
name: publish-build
path: dist/*

test-wheel:
needs: build-wheel
runs-on: ubuntu-latest
strategy:
matrix:
python-version: ["3.10", "3.11", "3.12", "3.13"]
steps:
- name: Set up Python
uses: actions/setup-python@v5
with:
python-version: ${{ matrix.python-version }}

- name: Validate version matches tag
- name: Download build artifacts
uses: actions/download-artifact@v4
with:
name: publish-build

- name: List files
run: ls -l

- name: Install wheel
run: |
VERSION_IN_FILE=$(grep '^version = ' pyproject.toml | sed 's/version = "\(.*\)"/\1/')
TAG_VERSION="${{ steps.version.outputs.version }}"
if [ "$VERSION_IN_FILE" != "$TAG_VERSION" ]; then
echo "❌ Version mismatch: pyproject.toml=$VERSION_IN_FILE, tag=$TAG_VERSION"
pip install --upgrade pip
pip install poetry==1.8.2
WHEEL=$(ls nemoguardrails-*.whl)
pip install "${WHEEL}[server]"
Comment on lines +90 to +95
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Unnecessary poetry install in test job

pip install poetry==1.8.2 is executed here but poetry is never invoked in the test-wheel job — only pip is used to install the wheel and run the server tests. Removing this reduces install time across all four matrix Python versions.

Suggested change
- name: Install wheel
run: |
VERSION_IN_FILE=$(grep '^version = ' pyproject.toml | sed 's/version = "\(.*\)"/\1/')
TAG_VERSION="${{ steps.version.outputs.version }}"
if [ "$VERSION_IN_FILE" != "$TAG_VERSION" ]; then
echo "❌ Version mismatch: pyproject.toml=$VERSION_IN_FILE, tag=$TAG_VERSION"
pip install --upgrade pip
pip install poetry==1.8.2
WHEEL=$(ls nemoguardrails-*.whl)
pip install "${WHEEL}[server]"
- name: Install wheel
run: |
pip install --upgrade pip
WHEEL=$(ls nemoguardrails-*.whl)
pip install "${WHEEL}[server]"
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/publish-pypi-approval.yml
Line: 90-95

Comment:
**Unnecessary `poetry` install in test job**

`pip install poetry==1.8.2` is executed here but `poetry` is never invoked in the `test-wheel` job — only `pip` is used to install the wheel and run the server tests. Removing this reduces install time across all four matrix Python versions.

```suggestion
      - name: Install wheel
        run: |
          pip install --upgrade pip
          WHEEL=$(ls nemoguardrails-*.whl)
          pip install "${WHEEL}[server]"
```

How can I resolve this? If you propose a fix, please make it concise.


- name: Start server in the background
run: |
nemoguardrails server &
echo "SERVER_PID=$!" >> $GITHUB_ENV

- name: Wait for server to be up
run: |
echo "Waiting for server to start..."
for i in {1..30}; do
if curl --output /dev/null --silent --head --fail http://localhost:8000; then
echo "Server is up!"
break
else
echo "Waiting..."
sleep 1
fi
done
Comment on lines +102 to +113
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Wait loop exits silently if server never starts

If the server fails to respond within all 30 iterations, the loop ends with exit code 0 and the step is marked as passed. The failure is only detected in the subsequent Check server status step, which can produce confusing output (it will report an HTTP error code rather than a timeout). Consider adding an explicit failure after the loop:

Suggested change
- name: Wait for server to be up
run: |
echo "Waiting for server to start..."
for i in {1..30}; do
if curl --output /dev/null --silent --head --fail http://localhost:8000; then
echo "Server is up!"
break
else
echo "Waiting..."
sleep 1
fi
done
- name: Wait for server to be up
run: |
echo "Waiting for server to start..."
for i in {1..30}; do
if curl --output /dev/null --silent --head --fail http://localhost:8000; then
echo "Server is up!"
exit 0
else
echo "Waiting... ($i/30)"
sleep 1
fi
done
echo "Server did not start within 30 seconds." >&2
exit 1
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/publish-pypi-approval.yml
Line: 102-113

Comment:
**Wait loop exits silently if server never starts**

If the server fails to respond within all 30 iterations, the loop ends with exit code `0` and the step is marked as passed. The failure is only detected in the subsequent `Check server status` step, which can produce confusing output (it will report an HTTP error code rather than a timeout). Consider adding an explicit failure after the loop:

```suggestion
      - name: Wait for server to be up
        run: |
          echo "Waiting for server to start..."
          for i in {1..30}; do
            if curl --output /dev/null --silent --head --fail http://localhost:8000; then
              echo "Server is up!"
              exit 0
            else
              echo "Waiting... ($i/30)"
              sleep 1
            fi
          done
          echo "Server did not start within 30 seconds." >&2
          exit 1
```

How can I resolve this? If you propose a fix, please make it concise.


- name: Check server status
run: |
RESPONSE_CODE=$(curl -s -o /dev/null -w "%{http_code}" http://localhost:8000/v1/rails/configs)
if [ "$RESPONSE_CODE" -ne 200 ]; then
echo "Server responded with code $RESPONSE_CODE."
exit 1
fi
echo "✅ Version validated: $VERSION_IN_FILE matches tag $TAG_VERSION"

- name: Download artifact
- name: Stop server
if: ${{ success() }}
run: kill $SERVER_PID
Comment on lines +123 to +125
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Server process not cleaned up on test failure

if: ${{ success() }} means the kill $SERVER_PID step is skipped whenever an earlier step fails (e.g., Check server status exits non-zero). On a self-hosted runner or a long-lived environment this could leave a dangling nemoguardrails server process. Using always() ensures cleanup regardless of outcome.

Suggested change
- name: Stop server
if: ${{ success() }}
run: kill $SERVER_PID
- name: Stop server
if: always()
run: kill $SERVER_PID
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/publish-pypi-approval.yml
Line: 123-125

Comment:
**Server process not cleaned up on test failure**

`if: ${{ success() }}` means the `kill $SERVER_PID` step is skipped whenever an earlier step fails (e.g., `Check server status` exits non-zero). On a self-hosted runner or a long-lived environment this could leave a dangling `nemoguardrails server` process. Using `always()` ensures cleanup regardless of outcome.

```suggestion
      - name: Stop server
        if: always()
        run: kill $SERVER_PID
```

How can I resolve this? If you propose a fix, please make it concise.


publish-pypi:
needs: test-wheel
runs-on: ubuntu-latest
environment:
name: pypi-production
url: https://pypi.org/project/nemoguardrails/
permissions:
contents: write
id-token: write
attestations: write
steps:
- name: Checkout tag
uses: actions/checkout@v4
with:
ref: v${{ inputs.version }}
fetch-depth: 0

- name: Download build artifacts
uses: actions/download-artifact@v4
with:
name: ${{ steps.version.outputs.artifact_name }}
name: publish-build
path: dist
github-token: ${{ secrets.GITHUB_TOKEN }}
repository: ${{ github.repository }}
run-id: ${{ github.event.workflow_run.id }}

- name: List files
run: ls -la dist/

- name: Generate artifact attestation
uses: actions/attest-build-provenance@v2
with:
subject-path: dist/*

- name: Publish to PyPI
uses: pypa/gh-action-pypi-publish@release/v1
with:
verbose: true
packages-dir: dist/
attestations: true

- name: Create GitHub Release
- name: Create GitHub release
env:
GH_TOKEN: ${{ github.token }}
run: |
TAG_NAME="${{ steps.version.outputs.tag }}"

git config --global user.name "github-actions[bot]"
git config --global user.email "github-actions[bot]@users.noreply.github.com"
TAG_NAME="v${{ inputs.version }}"

CHANGELOG_SECTION=$(awk -v version="${{ steps.version.outputs.version }}" '
CHANGELOG_SECTION=$(awk -v version="${{ inputs.version }}" '
/^## \[/ {
if (found) exit
if ($0 ~ "\\[" version "\\]") {
Expand All @@ -101,16 +183,16 @@ jobs:
echo "$CHANGELOG_SECTION" > release_notes.md

if gh release view "$TAG_NAME" --repo ${{ github.repository }} >/dev/null 2>&1; then
echo "ℹ️ Release $TAG_NAME already exists, skipping creation"
echo "Release $TAG_NAME already exists, skipping creation"
else
if gh release create "$TAG_NAME" \
--draft \
--title "$TAG_NAME" \
--notes-file release_notes.md \
--repo ${{ github.repository }}; then
echo "Release $TAG_NAME created successfully"
echo "Release $TAG_NAME created successfully"
else
echo "Failed to create release $TAG_NAME" >&2
echo "Failed to create release $TAG_NAME" >&2
rm -f release_notes.md
exit 1
fi
Expand Down
29 changes: 0 additions & 29 deletions .github/workflows/publish-wheel.yml

This file was deleted.

Loading