Skip to content

quality-debt: .github/workflows/e2e.yml — PR #1280 review feedback (high) #1288

@superdav42

Description

@superdav42

Unactioned Review Feedback

Source PR: #1280
File: .github/workflows/e2e.yml
Reviewers: coderabbit
Findings: 1
Max severity: high


HIGH: coderabbit (coderabbitai[bot])

File: .github/workflows/e2e.yml:468
⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE=".github/workflows/e2e.yml"
rg -n 'if \[ -z "\$PAYPAL_SANDBOX_CLIENT_ID" \] \|\| \[ -z "\$PAYPAL_SANDBOX_CLIENT_SECRET" \ ]; then|if \[ -z "\$PAYPAL_SANDBOX_CLIENT_SECRET" \ ]; then' "$FILE"

Repository: Ultimate-Multisite/ultimate-multisite

Length of output: 379


🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE=".github/workflows/e2e.yml"

# Show the exact gate block around the referenced line range
nl -ba "$FILE" | sed -n '420,470p'

# Find all occurrences of PayPal secret variables
rg -n 'PAYPAL_SANDBOX_CLIENT_(ID|SECRET)' "$FILE" || true

# Look for the specific conditional check patterns (simple, no regex escapes)
rg -n 'if \[ -z "\$PAYPAL_SANDBOX_CLIENT_' "$FILE" || true

Repository: Ultimate-Multisite/ultimate-multisite

Length of output: 124


🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE=".github/workflows/e2e.yml"

echo "=== lines 420-470 (with numbers) ==="
awk 'NR>=420 && NR<=470 {printf "%d\t%s\n", NR, $0}' "$FILE"

echo
echo "=== occurrences of both secret names ==="
rg -n 'PAYPAL_SANDBOX_CLIENT_(ID|SECRET)' "$FILE" || true

echo
echo "=== occurrences of the specific skip gate (secret missing) ==="
rg -n 'Skipping PayPal tests|PAYPAL_SANDBOX_CLIENT_SECRET not available|PAYPAL_SANDBOX_CLIENT_ID.*not available|if \[ -z "\$PAYPAL_SANDBOX_CLIENT_' "$FILE" || true

Repository: Ultimate-Multisite/ultimate-multisite

Length of output: 3368


Gate PayPal tests on both required secrets

.github/workflows/e2e.yml only skips when PAYPAL_SANDBOX_CLIENT_SECRET is empty; if PAYPAL_SANDBOX_CLIENT_ID is missing (empty) but the secret is present, the job will proceed and fail the tests instead of skipping.

Suggested fix
-          if [ -z "$PAYPAL_SANDBOX_CLIENT_SECRET" ]; then
+          if [ -z "$PAYPAL_SANDBOX_CLIENT_ID" ] || [ -z "$PAYPAL_SANDBOX_CLIENT_SECRET" ]; then
             echo "⏭️  Skipping PayPal tests: PAYPAL_SANDBOX_CLIENT_SECRET not available in this run."

(Also update the skip message to reference both PAYPAL_SANDBOX_CLIENT_ID and PAYPAL_SANDBOX_CLIENT_SECRET so it matches the new condition.)

📝 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.

          if [ -z "$PAYPAL_SANDBOX_CLIENT_ID" ] || [ -z "$PAYPAL_SANDBOX_CLIENT_SECRET" ]; then
            echo "⏭️  Skipping PayPal tests: PAYPAL_SANDBOX_CLIENT_SECRET not available in this run."
            echo ""
            echo "    The repo secrets PAYPAL_SANDBOX_CLIENT_ID and PAYPAL_SANDBOX_CLIENT_SECRET"
            echo "    must be configured for these tests to run. GitHub Actions does NOT expose"
            echo "    secrets to workflows triggered by pull_request events from forked"
            echo "    repositories — this is a security feature, not a misconfiguration."
            echo ""
            echo "    PayPal tests will run automatically when:"
            echo "      • this branch is merged to main (push event), or"
            echo "      • the PR is opened from a branch in the upstream Ultimate-Multisite repo"
            echo "        AND the PAYPAL_SANDBOX_* secrets have been configured at the repo level."
            echo ""
            echo "    To verify locally, run: gh secret list --repo Ultimate-Multisite/ultimate-multisite"
            exit 0
          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/e2e.yml around lines 437 - 452, The conditional that gates
PayPal tests only checks PAYPAL_SANDBOX_CLIENT_SECRET; change the if-condition
to test both PAYPAL_SANDBOX_CLIENT_ID and PAYPAL_SANDBOX_CLIENT_SECRET (e.g.,
test -z "$PAYPAL_SANDBOX_CLIENT_ID" || -z "$PAYPAL_SANDBOX_CLIENT_SECRET" or a
combined -z check) so the job skips when either secret is missing, and update
the echo messages inside that block to mention both PAYPAL_SANDBOX_CLIENT_ID and
PAYPAL_SANDBOX_CLIENT_SECRET to reflect the new gating logic.

View comment



Auto-generated by quality-feedback-helper.sh scan-merged. Review each finding and either fix the code or dismiss with a reason.


aidevops.sh v3.19.5 automated scan.

Metadata

Metadata

Assignees

Labels

origin:workerAuto-created by pulse labelless backfill (t2112)priority:highHigh severity — significant quality issuequality-debtUnactioned review feedback from merged PRssecuritySecurity-sensitive issue or changesource:review-feedbackAuto-created by quality-feedback-helper.sh

Type

No type
No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions