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
1 change: 0 additions & 1 deletion .github/workflows/_healthcheck_vm.yml
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ on:

jobs:
check-status-and-maybe-shutdown:
environment: main
runs-on: ${{ inputs.vm }}
outputs:
status: ${{ steps.status.outputs.main }}
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/build-test-publish-wheel.yml
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ jobs:
python-package: nemo_rl
packaging: uv
secrets:
TWINE_USERNAME: ${{ secrets.TWINE_USERNAME }}
TWINE_PASSWORD: ${{ secrets.TWINE_PASSWORD }}
TWINE_USERNAME: __token__
TWINE_PASSWORD: ${{ (github.ref == 'refs/heads/main' || startsWith(github.ref, 'refs/heads/r')) && secrets.SVC_PYPI_TOKEN || secrets.SVC_PYPI_TEST_TOKEN }}
SLACK_WEBHOOK: ${{ secrets.SLACK_RELEASE_ENDPOINT }}
SLACK_WEBHOOK_ADMIN: ${{ secrets.SLACK_WEBHOOK_ADMIN }}
7 changes: 3 additions & 4 deletions .github/workflows/cicd-main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ jobs:
- name: Checkout repository
uses: actions/checkout@v4
with:
submodules: 'recursive'
submodules: "recursive"
- name: Install uv
uses: astral-sh/setup-uv@v5
with:
Expand Down Expand Up @@ -234,7 +234,6 @@ jobs:
if: ${{ contains('docs L0 L1 L2', needs.pre-flight.outputs.test_level) }}
runs-on: ${{ matrix.runner }}
name: ${{ matrix.is_optional && 'PLEASEFIXME_' || '' }}${{ matrix.script }}
environment: nemo-ci
steps:
- name: Checkout
uses: actions/checkout@v4
Expand Down Expand Up @@ -286,7 +285,6 @@ jobs:
runs-on: ${{ matrix.runner }}
if: ${{ contains('L1 L2', needs.pre-flight.outputs.test_level) }}
name: ${{ matrix.is_optional && 'PLEASEFIXME_' || '' }}${{ matrix.script }}
environment: nemo-ci
steps:
- name: Checkout
uses: actions/checkout@v4
Expand Down Expand Up @@ -334,6 +332,8 @@ jobs:
)
)
}}


CI_SKIP: ${{ github.event.label.name == 'Skip CICD' }}
TEST_LEVEL: ${{ needs.pre-flight.outputs.test_level }}
run: |
Expand All @@ -346,7 +346,6 @@ jobs:
name: Notify nightly test failure
runs-on: ubuntu-latest
needs: [CI_QA_Gate]
environment: main
if: ${{ always() && github.event_name == 'schedule' && needs.CI_QA_Gate.result == 'failure' }}
steps:
- name: Send Slack notification
Expand Down
3 changes: 1 addition & 2 deletions .github/workflows/healthcheck_vms.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,14 @@
name: VM Health Check and Reboot
on:
schedule:
- cron: '0 7 * * *'
- cron: "0 7 * * *"
workflow_dispatch:

jobs:
pre-flight:
runs-on: ubuntu-latest
outputs:
list-of-vms: ${{ steps.main.outputs.main }}
environment: main
steps:
- name: Get list of VMs
id: main
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/release.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ jobs:
create-gh-release: ${{ inputs.create-gh-release }}
packaging: uv
secrets:
TWINE_USERNAME: ${{ secrets.TWINE_USERNAME }}
TWINE_PASSWORD: ${{ secrets.TWINE_PASSWORD }}
TWINE_USERNAME: __token__
TWINE_PASSWORD: ${{ (github.ref == 'refs/heads/main' || startsWith(github.ref, 'refs/heads/r')) && secrets.SVC_PYPI_TOKEN || secrets.SVC_PYPI_TEST_TOKEN }}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Silent SVC_PYPI_TEST_TOKEN fallback if SVC_PYPI_TOKEN is empty.

A known limitation of the &&/|| ternary pattern is that the value after && must not be falsy; otherwise, the || branch is taken incorrectly. In GitHub Actions, falsy values include false, 0, -0, "", '', and null — which covers an unset or empty secret.

If SVC_PYPI_TOKEN is ever missing or accidentally revoked, the expression on main/release branches silently resolves to SVC_PYPI_TEST_TOKEN and the release is published to TestPyPI instead of production PyPI — with no error or warning. Since dry-run can be false in this workflow, this is a real risk.

Consider asserting the secret is non-empty in a pre-publish step, or using fromJSON indexing to avoid the silent fallback:

💡 Alternative ternary pattern that avoids the silent fallback
-      TWINE_PASSWORD: ${{ (github.ref == 'refs/heads/main' || startsWith(github.ref, 'refs/heads/r')) && secrets.SVC_PYPI_TOKEN || secrets.SVC_PYPI_TEST_TOKEN }}
+      TWINE_PASSWORD: ${{ fromJSON(format('["{0}","{1}"]', secrets.SVC_PYPI_TEST_TOKEN, secrets.SVC_PYPI_TOKEN))[github.ref == 'refs/heads/main' || startsWith(github.ref, 'refs/heads/r')] }}

The fromJSON array-index approach uses true1 / false0 coercion, so it unconditionally selects the correct token without the "falsy middle operand" pitfall.

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

In @.github/workflows/release.yaml at line 51, The TWINE_PASSWORD expression in
release.yaml uses the &&/|| pattern which silently falls back to
SVC_PYPI_TEST_TOKEN if SVC_PYPI_TOKEN is empty; change the workflow to ensure a
non-empty production token is selected by either adding a pre-publish assertion
step that fails when SVC_PYPI_TOKEN is unset/empty (check secrets.SVC_PYPI_TOKEN
and fail the job) or replace the expression with the fromJSON array-index
ternary pattern so TWINE_PASSWORD deterministically picks secrets.SVC_PYPI_TOKEN
for refs/heads/main or release branches and only uses
secrets.SVC_PYPI_TEST_TOKEN for non-release refs, referencing the TWINE_PASSWORD
variable and the SVC_PYPI_TOKEN / SVC_PYPI_TEST_TOKEN secret names when making
the change.

SLACK_WEBHOOK_ADMIN: ${{ secrets.SLACK_WEBHOOK_ADMIN }}
SLACK_WEBHOOK: ${{ secrets.SLACK_RELEASE_ENDPOINT }}
PAT: ${{ secrets.PAT }}
Loading