Skip to content

ADFA-3269 Bugfix for release.yml#1078

Merged
hal-eisen-adfa merged 1 commit intostagefrom
ADFA-3269-bugfix-2
Mar 13, 2026
Merged

ADFA-3269 Bugfix for release.yml#1078
hal-eisen-adfa merged 1 commit intostagefrom
ADFA-3269-bugfix-2

Conversation

@hal-eisen-adfa
Copy link
Collaborator

Remove duplicate variant
Add debug for token and message

@hal-eisen-adfa hal-eisen-adfa merged commit 78b1f8c into stage Mar 13, 2026
0 of 2 checks passed
@hal-eisen-adfa hal-eisen-adfa deleted the ADFA-3269-bugfix-2 branch March 13, 2026 23:43
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 13, 2026

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 4fc42928-6052-45b1-89f2-4d90909b7b1d

📥 Commits

Reviewing files that changed from the base of the PR and between 483062d and f417de6.

📒 Files selected for processing (2)
  • .github/workflows/release.yml
  • scripts/cloudflare-r2-upload.py

📝 Walkthrough

Release Notes

Changes

  • Token verification: Added a check to ensure TELEGRAM_TOKEN is set before proceeding, with logging of token length and masked preview (first and last 3 characters)
  • Simplified upload script: Removed the variant argument from the Cloudflare R2 upload script invocation; now only the APK file path is passed
  • Filename normalization: APK filenames are now derived solely from the basename of the APK path, without variant-specific modifications
  • Debug logging: Added od -c output for Telegram messages to aid in pipeline observability; maintained 4096-character message limit

⚠️ Risks and Best Practice Violations

Critical Issue - Filename Collisions:

  • The workflow matrix builds two APK variants (v8 64-bit and v7 32-bit) in parallel, but both upload to Cloudflare R2 with identical filenames
  • The second variant to complete will silently overwrite the first variant's upload, resulting in data loss
  • Mitigation required: Include variant identifier in the uploaded filename to ensure both builds are preserved

Security Risk - Token Exposure in Logs:

  • Even though masked, logging any portion of TELEGRAM_TOKEN in GitHub Actions logs violates security best practices
  • Recommendation: Remove token logging entirely or use GitHub's built-in secret masking only, and avoid any echo or printf statements that reference secrets

Code Quality:

  • Debug output (od -c on line 719) is production-level code and should be removed or moved to a conditional debug mode
  • No validation that the Telegram message was successfully sent; the curl command silently fails if the API returns an error

Walkthrough

This PR simplifies the APK upload workflow by removing the variant argument from the Cloudflare R2 upload script and adjusting downstream filename construction accordingly. The release workflow now derives APK_FILENAME directly from the file basename, adds Telegram token verification, and updates messaging flows to align with the new naming convention.

Changes

Cohort / File(s) Summary
Release Workflow
.github/workflows/release.yml
Added Telegram token verification step; removed variant argument from upload script invocation; simplified APK_FILENAME to use basename only (no variant suffix); updated Telegram and Slack messages to align with new filename format; added debug output for token length and message content.
Upload Script
scripts/cloudflare-r2-upload.py
Removed support for the secondary "variant" positional argument; updated script to require only file_path parameter; simplified filename construction to directly use the file basename without modification; updated usage text.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • MadaraMods
  • Daniel-ADFA

Poem

🐰 A variant took its final bow,
The upload path simplifies now,
Token checks guard the way,
Filenames trimmed, clean and gray,
One argument left—hooray! ✨

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch ADFA-3269-bugfix-2
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

Tip

CodeRabbit can scan for known vulnerabilities in your dependencies using OSV Scanner.

OSV Scanner will automatically detect and report security vulnerabilities in your project's dependencies. No additional configuration is required.

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