Skip to content

Harden the Verify Step Against Unset Auth Tokens - #106

Merged
ptr727 merged 1 commit into
developfrom
harden-verify-step
Aug 24, 2026
Merged

Harden the Verify Step Against Unset Auth Tokens#106
ptr727 merged 1 commit into
developfrom
harden-verify-step

Conversation

@ptr727

@ptr727 ptr727 commented Aug 24, 2026

Copy link
Copy Markdown
Owner

Fixes qodo's finding on #105 (production, where these are legitimately absent). $SITE_AUTH_TOKEN_ID/$SITE_AUTH_TOKEN are expanded under set -u without a default. GitHub's own env: block always defines a mapped key even when its value is empty, so this should already be safe, but safe expansion (${VAR:-}) removes the doubt at zero cost, right before this path's first production dispatch.

Verified

scripts/docker_lint.py --linter actionlint --linter editorconfig-checker and scripts/prose_lint.py --diff origin/main pass clean.

Summary by CodeRabbit

  • Bug Fixes
    • Improved deployment verification by handling missing site authentication credentials gracefully.
    • Live URL checks no longer fail solely because authentication variables are unset.

The verify step runs under set -u and expanded $SITE_AUTH_TOKEN_ID/
$SITE_AUTH_TOKEN directly. GitHub Actions' env: block always defines
a mapped key, even empty, so the hub task's own unconditional env
mapping should already make this safe on production. Safe expansion
removes the doubt regardless, at no cost, right before this path's
first production dispatch.
@coderabbitai

coderabbitai Bot commented Aug 24, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 2b20c9e9-d72f-4057-b3a1-ffeaaf28879e

📥 Commits

Reviewing files that changed from the base of the PR and between 0b2f500 and 5336d95.

📒 Files selected for processing (1)
  • .github/actions/deploy/action.yml

Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review.


📝 Walkthrough

Walkthrough

The deploy action now defaults unset site authentication variables to empty strings before it runs live URL checks.

Changes

Deploy verification

Layer / File(s) Summary
Default authentication variables
.github/actions/deploy/action.yml
The verify command uses empty values when SITE_AUTH_TOKEN_ID or SITE_AUTH_TOKEN is unset before invoking checks/check-live-urls.sh.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Merge Risk: ⚪ Minimal · up to 5336d

This change makes verification safe when authentication tokens are absent without changing the normal authenticated path. No actionable merge-blocking risk remains beyond normal checks and review.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes hardening the Verify step against unset authentication tokens.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (1 skipped: 1 unsupported.)
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch harden-verify-step

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

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Harden verify step against unset auth tokens

🐞 Bug fix ⚙️ Configuration changes 🕐 Less than 10 minutes

Grey Divider

AI Description

• Prevent verify step failures when auth token env vars are unset under set -u.
• Use safe bash expansions for SITE_AUTH_TOKEN_ID and SITE_AUTH_TOKEN.
• Keep production dispatch behavior unchanged while removing ambiguity around empty env mapping.
Diagram

graph TD
  A["Composite deploy action"] --> B["Verify step"] --> C["check-live-urls.sh"]
  D("SITE_AUTH_TOKEN(_ID)") --> B
Loading
High-Level Assessment

The chosen approach (using ${VAR:-}) is the simplest and most robust hardening for set -u shells, independent of GitHub Actions env-mapping behavior. Alternatives like defining explicit defaults in the action env: block or adding conditional logic would be more verbose without improving correctness for this case.

Files changed (1) +1 / -1

Bug fix (1) +1 / -1
action.ymlSafely expand optional auth token env vars in verify step +1/-1

Safely expand optional auth token env vars in verify step

• Updates the verify step to use safe bash expansion ('${SITE_AUTH_TOKEN_ID:-}', '${SITE_AUTH_TOKEN:-}') when exporting values to 'PANGOLIN_ACCESS_TOKEN_ID'/'PANGOLIN_ACCESS_TOKEN'. This prevents 'set -u' from failing when the source env vars are legitimately absent in production dispatch paths.

.github/actions/deploy/action.yml

@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Grey Divider

Tip of the day
💡 Did you know, you can switch off images and animations for a plain-text comment

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

@ptr727
ptr727 merged commit 1048c94 into develop Aug 24, 2026
8 checks passed
@ptr727
ptr727 deleted the harden-verify-step branch August 24, 2026 03:44
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