Conversation
📝 WalkthroughWalkthroughAdded a tag-conditional Changes
Sequence Diagram(s)sequenceDiagram
participant Build as Build workflow
participant Reusable as Azure sync workflow
participant GH as GitHub Releases API
participant Azure as Azure Blob Storage
Build->>Reusable: workflow_call(release_tag = github.ref_name) [on tag]
Reusable->>Reusable: validate AZURE_BLOB_SAS_URL secret
alt release_tag provided
Reusable->>Reusable: use inputs.release_tag
else no release_tag
Reusable->>GH: query latest release (gh)
GH-->>Reusable: latest tag
end
Reusable->>Azure: upload assets using SAS URL
Azure-->>Reusable: upload status
Reusable-->>Build: outputs (tag, status)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/sync-azure-storage.yml (1)
77-77:⚠️ Potential issue | 🟠 MajorRegex for SAS token extraction appears incorrect.
The sed pattern
's|[^?]*\(?\.*\)|\1|p'uses\.*which matches zero or more literal dots, not "everything after?". This will likely capture only?or?.sequences rather than the full SAS token.Proposed fix
- SAS_TOKEN=$(echo "$SAS_URL" | sed -n 's|[^?]*\(?\.*\)|\1|p') + SAS_TOKEN=$(echo "$SAS_URL" | sed -n 's|[^?]*\(?.*\)|\1|p')Or more simply using parameter expansion:
- SAS_TOKEN=$(echo "$SAS_URL" | sed -n 's|[^?]*\(?\.*\)|\1|p') + SAS_TOKEN="${SAS_URL#*\?}" + SAS_TOKEN="?${SAS_TOKEN}"
🤖 Fix all issues with AI agents
In @.github/workflows/sync-azure-storage.yml:
- Around line 5-12: The workflow currently defines a required input release_tag
only under workflow_call, but workflow_dispatch has no inputs so triggering
manually yields an empty inputs.release_tag and later fails; add a release_tag
input to the workflow_dispatch invocation (match description/required/type to
the existing release_tag) or alternatively guard steps that use
inputs.release_tag, updating the workflow_dispatch section to declare
release_tag (string, required true) so inputs.release_tag is populated when
using workflow_dispatch.
🧹 Nitpick comments (3)
.github/workflows/sync-azure-storage.yml (2)
33-39: Missing validation for emptyrelease_tag.If
inputs.release_tagis empty or invalid, the workflow will proceed and fail at thegh release downloadstep with an unclear error. Add early validation to provide a clear error message.Proposed fix
- name: Determine release tag id: release_info run: | # From workflow_call input RELEASE_TAG="${{ inputs.release_tag }}" + if [ -z "$RELEASE_TAG" ]; then + echo "::error::release_tag input is required but was empty" + exit 1 + fi echo "tag=${RELEASE_TAG}" >> $GITHUB_OUTPUT echo "Release tag: ${RELEASE_TAG}"
79-79: SAS token stored in plaintext file without restricted permissions.The SAS token is written to
/tmp/sas_token.txtwith default permissions, potentially readable by other processes. While GitHub Actions runners are ephemeral, it's still a good practice to restrict access.Proposed fix
# Store token as file to avoid issues with special characters + umask 077 echo "$SAS_TOKEN" > /tmp/sas_token.txt.github/workflows/build.yml (1)
288-295: Sync job may run even when builds have failed.The
sync-azurejob depends onbuild-summary, which hasif: always()(line 269). Whenbuild-summaryfails (due to a failed build), jobs that depend on it are skipped by default. However, the intent seems to be "sync only after successful builds."For clarity and to explicitly guard against syncing after failed builds, consider adding a success check:
Proposed fix
sync-azure: name: Sync to Azure Storage needs: build-summary - if: startsWith(github.ref, 'refs/tags/v') + if: startsWith(github.ref, 'refs/tags/v') && needs.build-summary.result == 'success' uses: ./.github/workflows/sync-azure-storage.yml with: release_tag: ${{ github.ref_name }} secrets: inherit
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
.github/workflows/sync-azure-storage.yml (3)
30-36:⚠️ Potential issue | 🟡 MinorSecret value expanded in shell conditional.
Directly embedding
${{ secrets.AZURE_BLOB_SAS_URL }}in the shell script expands the secret into the script text. While GitHub masks secrets in logs, this pattern can leak secrets in certain error scenarios. Use an environment variable instead.Proposed fix
- name: Validate SAS URL secret + env: + AZURE_BLOB_SAS_URL: ${{ secrets.AZURE_BLOB_SAS_URL }} run: | - if [ -z "${{ secrets.AZURE_BLOB_SAS_URL }}" ]; then + if [ -z "$AZURE_BLOB_SAS_URL" ]; then echo "::error::AZURE_BLOB_SAS_URL secret not found. Please configure it in GitHub Secrets." exit 1 fi echo "SAS URL secret found"
88-91:⚠️ Potential issue | 🔴 CriticalRegex bug:
\.*matches literal dots, not any characters.The sed pattern
's|[^?]*\(?\.*\)|\1|p'uses\.*which matches zero or more literal.characters, not "any character". This will fail to extract the full SAS token if it contains characters other than dots after the?.Proposed fix
# Get the SAS token (everything after ?) - SAS_TOKEN=$(echo "$SAS_URL" | sed -n 's|[^?]*\(?\.*\)|\1|p') + SAS_TOKEN=$(echo "$SAS_URL" | sed -n 's|[^?]*\(?.*\)|\1|p')
72-75:⚠️ Potential issue | 🟡 MinorUse environment variable for secret instead of direct expansion.
Same concern as the validation step—expand the secret via an
envblock rather than inline expansion.Proposed fix
- name: Parse SAS URL and extract storage info id: sas_info + env: + SAS_URL: ${{ secrets.AZURE_BLOB_SAS_URL }} run: | - SAS_URL="${{ secrets.AZURE_BLOB_SAS_URL }}" + # SAS_URL is provided via env
🧹 Nitpick comments (3)
.github/workflows/sync-azure-storage.yml (3)
38-51: Add error handling forgh release view.If no releases exist in the repository,
gh release viewwill fail but the error may not be clear. Consider adding explicit error handling.Proposed improvement
- name: Determine release tag id: release_info run: | # From workflow_call input, or fetch latest for manual dispatch if [ -n "${{ inputs.release_tag }}" ]; then RELEASE_TAG="${{ inputs.release_tag }}" else - RELEASE_TAG=$(gh release view --json tagName -q .tagName) + RELEASE_TAG=$(gh release view --json tagName -q .tagName) || { + echo "::error::No releases found in repository. Please create a release first or specify a release_tag." + exit 1 + } echo "No tag specified, using latest release: ${RELEASE_TAG}" fi echo "tag=${RELEASE_TAG}" >> $GITHUB_OUTPUT echo "Release tag: ${RELEASE_TAG}" env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
120-121: Unused variableBLOB_URL.
BLOB_URLis constructed but never used—the actual upload usesaz storage blob uploadwith--sas-token. Remove this dead code.Proposed fix
echo "Uploading: ${filename}" - # Build the blob URL with SAS token - BLOB_URL="https://${ACCOUNT}.blob.core.windows.net/${CONTAINER}/${BLOB_PATH}${filename}${SAS_TOKEN}" - # Upload using az storage blob upload with SAS authentication
197-198:if: success()is redundant.Steps run on success by default; the explicit
if: success()condition can be removed.Proposed fix
- name: Verify upload - if: success() run: |
Summary by CodeRabbit