DAT-21154: fix dry runs #435
Conversation
…nch in build command
…le in artifact release workflow
- Check if REPO_LIQUIBASE_NET_USER and REPO_LIQUIBASE_NET_PASSWORD are set - Display credential length for verification - Exit early if credentials are missing 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
…rtifacts for Maven Central
…instead of Maven command
…using environment variables for credentials
…and using wildcard for file attachments
…ects and adjusting module paths
… attachment process
…streamline artifact path detection
…gs in dry run process
|
Warning Rate limit exceeded@sayaliM0412 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 10 minutes and 18 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughUpdates two GitHub Actions workflows to unify artifact version resolution, make Maven invocations dry-run aware, produce versioned POM/sources/javadoc, collect dry-run artifacts into a separate path, and switch publishing to per-artifact handling for multi-module projects. Changes
Sequence Diagram(s)sequenceDiagram
participant WF as GH Actions Workflow
participant Maven as Maven
participant FS as Filesystem
participant Git as GitHub Release
rect rgba(220,240,255,0.5)
note right of WF: Start workflow — read dry_run & MAVEN_PROFILES
WF->>Maven: Resolve version (use dry_run_version if dry_run)\nRun release:prepare (dry-run uses -DdryRun=true)
Maven-->>WF: Produced artifacts in target/ (versioned)
WF->>FS: Create sources/javadoc jars\nCopy versioned POM(s)
alt dry_run
WF->>FS: Rename SNAPSHOT files -> dry-run version\nCollect artifacts -> /tmp/release-artifacts
WF->>Git: Attach /tmp/release-artifacts/* to draft release
else normal
WF->>Maven: Sign artifacts using resolved version
WF->>FS: Prepare per-artifact bundle layout
WF->>Git: Attach ./target/* artifacts and publish per-artifact bundles
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
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. Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.github/workflows/extension-attach-artifact-release.yml (1)
565-580: New artifact path handling for dry-run file attachment: verify files are present in target/ before glob expansion.The new "Set artifact path" step (lines 565-568) constructs the artifact path for use in the subsequent file attachment step (line 580). The glob pattern
${{ steps.set-artifact-path.outputs.ARTIFACT_PATH }}/target/*will expand to files in the target directory. However, there is no validation that:
- The target directory exists.
- Files are actually present before the glob is evaluated.
If the build or prior steps fail to produce artifacts, the
softprops/action-gh-releaseaction will fail due tofail_on_unmatched_files: true(line 576), which is correct for detecting issues but could mask root causes.Consider adding a step to verify artifacts exist:
- name: Verify artifacts exist if: ${{ inputs.dry_run == true }} run: | artifact_path="${{ steps.set-artifact-path.outputs.ARTIFACT_PATH }}/target" if [ ! -d "$artifact_path" ] || [ -z "$(ls -1 "$artifact_path" 2>/dev/null)" ]; then echo "Error: No artifacts found in $artifact_path" exit 1 fi echo "Found $(ls -1 "$artifact_path" | wc -l) artifacts"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.github/workflows/extension-attach-artifact-release.yml(7 hunks).github/workflows/extension-release-published.yml(2 hunks)
🔇 Additional comments (5)
.github/workflows/extension-attach-artifact-release.yml (5)
178-179: Maven dry-run flag correctly added for dry-run builds.The
-DdryRun=trueflag addition to the release:prepare command is appropriate and aligns with Maven release plugin semantics. This prevents uncommitted changes and tag creation during dry-run.
199-204: Unified artifact version retrieval with conditional logic is correct.The refactoring consolidates version resolution into a single step with proper branching: using
dry_run_versioninput whendry_run == true, and resolving from Maven otherwise. This improves maintainability and ensures consistency.
419-435: Build release artifacts step now generates complete artifact set and uses default branch for builds.The changes add source/javadoc JAR generation (line 429) and versioned POM copying (line 434), which aligns with the PR objective to enhance artifact generation. Using
github.event.repository.default_branch(line 424) is the correct approach for determining the main branch.
437-440: POM version set to snapshot version before dry-run build, but version coordination with release:prepare needs verification.Line 440 sets the POM version to
${{ inputs.dry_run_version }}-SNAPSHOT. However, the subsequentrelease:preparecommand at lines 446-448 uses-DreleaseVersion=${{ inputs.dry_run_version }}(non-SNAPSHOT). Verify that:
- The Maven release plugin correctly respects the
-DreleaseVersionoverride and produces artifacts with the intended version.- The version resolved at line 459 after the build matches expectations (should be
inputs.dry_run_versionnon-SNAPSHOT).This version manipulation is necessary for the workflow, but the coordination with the downstream
extension-release-published.ymlworkflow (which renames SNAPSHOT artifacts) should be validated end-to-end.
538-543: Sign Files for Draft Release (dry-run) correctly uses Maven-resolved version after POM version was set.After the POM version is set at line 440 and the build completes, the signing step correctly retrieves the actual project version from Maven (line 538) to ensure signed artifacts match generated files. This approach is sound and resolves the signing version from the build state.
* fix: add processAllModules flag to mvn versions:set command for dry_run version * fix: enhance artifact processing for multi-module projects in dry run workflow * fix: add artifact collection step for dry-run in release workflow
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/extension-attach-artifact-release.yml (1)
450-452: Branch detection fails in detached HEAD state (GitHub Actions default checkout).
git rev-parse --abbrev-ref HEADreturns"HEAD"when the repository is in detached HEAD state, which is the default behavior for GitHub Actions checkouts. This causesbuildNumber=HEADinstead of a meaningful branch name.Replace with GitHub's built-in variables to properly detect the branch:
- CURRENT_BRANCH=$(git rev-parse --abbrev-ref HEAD) - echo "Using current branch: $CURRENT_BRANCH" - mvn clean install -DskipTests -P '${{ inputs.mavenProfiles }}' -DbuildNumber=$CURRENT_BRANCH + CURRENT_BRANCH="${{ github.head_ref || github.ref_name }}" + echo "Using current branch: $CURRENT_BRANCH" + mvn clean install -DskipTests -P '${{ inputs.mavenProfiles }}' -DbuildNumber=$CURRENT_BRANCHThis ensures the correct branch/tag is passed for both pull requests (
github.head_ref) and direct branch/tag pushes (github.ref_name).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.github/workflows/extension-attach-artifact-release.yml(7 hunks).github/workflows/extension-release-published.yml(4 hunks)
🧰 Additional context used
🪛 GitHub Check: CodeQL
.github/workflows/extension-release-published.yml
[warning] 347-347: Code injection
Potential code injection in ${{ inputs.dry_run_version }}, which may be controlled by an external user.
[warning] 361-361: Code injection
Potential code injection in ${{ inputs.dry_run_version }}, which may be controlled by an external user.
.github/workflows/extension-attach-artifact-release.yml
[warning] 178-178: Code injection
Potential code injection in ${{ inputs.dry_run_version }}, which may be controlled by an external user.
[warning] 178-178: Code injection
Potential code injection in ${{ inputs.mavenProfiles }}, which may be controlled by an external user.
[warning] 199-199: Code injection
Potential code injection in ${{ inputs.dry_run }}, which may be controlled by an external user.
[warning] 200-200: Code injection
Potential code injection in ${{ inputs.dry_run_version }}, which may be controlled by an external user.
[warning] 420-420: Code injection
Potential code injection in ${{ github.event.inputs.liquibaseVersion }}, which may be controlled by an external user.
[warning] 420-420: Code injection
Potential code injection in ${{ inputs.mavenProfiles }}, which may be controlled by an external user.
[warning] 424-424: Code injection
Potential code injection in ${{ github.event.repository.default_branch }}, which may be controlled by an external user.
[warning] 426-426: Code injection
Potential code injection in ${{ inputs.mavenProfiles }}, which may be controlled by an external user.
[warning] 440-440: Code injection
Potential code injection in ${{ inputs.dry_run_version }}, which may be controlled by an external user.
[warning] 447-447: Code injection
Potential code injection in ${{ inputs.dry_run_version }}, which may be controlled by an external user.
[warning] 447-447: Code injection
Potential code injection in ${{ inputs.mavenProfiles }}, which may be controlled by an external user.
[warning] 452-452: Code injection
Potential code injection in ${{ inputs.mavenProfiles }}, which may be controlled by an external user.
[warning] 568-568: Code injection
Potential code injection in ${{ inputs.artifactPath || '.' }}, which may be controlled by an external user.
[warning] 575-575: Code injection
Potential code injection in ${{ steps.set-artifact-path.outputs.ARTIFACT_PATH }}, which may be controlled by an external user.
🔇 Additional comments (9)
.github/workflows/extension-attach-artifact-release.yml (5)
565-575: Verify that artifact path is constrained to prevent injection or path traversal.Lines 575 use the user-provided
ARTIFACT_PATHdirectly in afindcommand without validation. While the expression at line 568 is safe (variable assignment), the downstream use at line 575 in a shell command could be exploited ifartifactPathcontains special characters or path traversal sequences.Verify one of the following:
- Documented constraint: Confirm that workflow documentation or code comments explicitly restrict
artifactPathto relative paths within the repository (e.g., reject../,~, or absolute paths).- Input validation: Add validation to reject unsafe paths before line 575:
# Validate artifactPath is relative and doesn't escape the repo if [[ "$ARTIFACT_PATH" =~ ^\.\./ ]] || [[ "$ARTIFACT_PATH" = /* ]]; then echo "::error::artifactPath must be a relative path within the repository" exit 1 fi
437-461: Verify dry-run versioning strategy—artifacts may retain-SNAPSHOTsuffix inconsistently with PR objectives.The PR description states: "For dry runs: setting the POM version to a dry-run version with
-SNAPSHOT, then renaming files and updating POM contents to remove-SNAPSHOTfor consistency." However, the code does not appear to rename artifacts or strip-SNAPSHOTafter the build.Flow trace for dry-run:
- Line 440: POM version set to
dry_run_version-SNAPSHOT- Line 446–448:
release:prepare -DdryRun=true -DreleaseVersion=${{ inputs.dry_run_version }}- Line 459:
project.versionevaluates todry_run_version-SNAPSHOT- Line 460: POM copied as
${ARTIFACT_ID}-${VERSION}.pom(includes-SNAPSHOT)- Line 575: All artifacts collected with
-SNAPSHOTsuffixExpected behavior: Verify whether the final attached artifacts should:
- Keep
-SNAPSHOTsuffix (current behavior), or- Be renamed to remove
-SNAPSHOT(per PR description)If renaming is required, add a step between line 461 and 522 to rename artifacts in
target/before signing and collection.
199-204: Unification of artifact version retrieval looks good.The conditional logic correctly branches on
dry_runto select the appropriate version source. This aligns with PR objectives and eliminates duplication.
419-435: Improved branch handling and artifact generation for release builds.The changes correctly use
github.event.repository.default_branchfor release builds instead of local branch detection, which is more reliable. Addition of source/javadoc jar generation and versioned POM copying enables consistent multi-module artifact handling, as intended by the PR objectives.
538-543: Signing correctly uses evaluated project.version for artifact name consistency.The signing step evaluates
project.versionrather than using the input variable directly, ensuring signed artifacts match the built artifacts. The added diagnostic output is helpful for troubleshooting..github/workflows/extension-release-published.yml (4)
159-159: Wildcard download simplifies multi-module artifact handling.The change from a specific artifact-id pattern to
"*"is appropriate for multi-module projects. Verify that release assets contain only the intended artifacts (JARs, POMs, signatures, checksums) and no extraneous files.
174-193: Multi-module artifact processing is well-structured.The iteration over JAR files with proper exclusion logic and per-artifact directory structure creation follows Maven conventions and handles multi-module projects cleanly. The "-full" filter and glob-based file copy are appropriate safeguards.
290-290: Snapshot publishing correctly enabled for dry-run repository.Enabling snapshots on the dry-run repository aligns with the PR objective to allow dry-run versions to be deployed for testing before actual release. This is the correct configuration.
361-384: Per-artifact dry-run deployment properly structured; code injection risk requires input validation.The loop-based per-artifact deployment to the dry-run repository follows the correct pattern and properly handles sources, javadoc, and signature files. However, line 361's direct assignment of
${{ inputs.dry_run_version }}toversionand subsequent use in glob pattern*-${version}.jar(line 364) presents the same code injection risk as the previous section.If
dry_run_versioncontains glob metacharacters or special characters, the pattern match could fail silently or match unintended files.Recommend validating
dry_run_versioninput format (alphanumeric, dots, hyphens only) at workflow entry point to mitigate this risk.
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
| git reset HEAD~ --hard | ||
| mvn -B dependency:go-offline clean package -DskipTests=true ${{ inputs.extraMavenArgs }} -P '${{ inputs.mavenProfiles }}' | ||
| mvn -B release:clean release:prepare -DdryRun=true -Darguments="-Dmaven.javadoc.skip=true -Dmaven.test.skipTests=true -Dmaven.test.skip=true -Dmaven.deploy.skip=true" -DcheckModificationExcludeList=** -DignoreSnapshots=true -DreleaseVersion=${{ inputs.dry_run_version }} -DpushChanges=false -P "$MAVEN_PROFILES" -DscmServerId=liquibase | ||
| mvn -B dependency:go-offline clean package -DskipTests=true ${{ inputs.extraMavenArgs }} -P "$MAVEN_PROFILES" |
Check warning
Code scanning / CodeQL
Code injection Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 2 days ago
To fix this issue, we should avoid using the input directly as an inline expression within the shell command. Instead, we set the value of the untrusted input to an environment variable and use the environment variable via native shell variable expansion syntax (e.g., $EXTRA_MAVEN_ARGS) inside the script. This transformation prevents code injection attacks by leveraging the shell’s handling of environment variables, ensuring that special characters are not interpreted as executable commands. The change should be made in both spots using ${{ inputs.extraMavenArgs }}: the non-dry-run build (line 174) and the dry-run build (line 184). This requires adding an environment variable mapping for EXTRA_MAVEN_ARGS in both relevant job steps, and using $EXTRA_MAVEN_ARGS in the shell scripts instead of ${{ inputs.extraMavenArgs }}.
| @@ -168,10 +168,11 @@ | ||
| shell: bash | ||
| env: | ||
| MAVEN_PROFILES: ${{ inputs.mavenProfiles }} | ||
| EXTRA_MAVEN_ARGS: ${{ inputs.extraMavenArgs }} | ||
| run: | | ||
| mvn -B release:clean release:prepare -Darguments="-Dmaven.javadoc.skip=true -Dmaven.test.skipTests=true -Dmaven.test.skip=true -Dmaven.deploy.skip=true" -DcheckModificationExcludeList=** -DignoreSnapshots=true -DreleaseVersion=${{ github.event.inputs.liquibaseVersion }} -DpushChanges=false -P "$MAVEN_PROFILES" -DscmServerId=liquibase | ||
| git reset HEAD~ --hard | ||
| mvn -B dependency:go-offline clean package -DskipTests=true ${{ inputs.extraMavenArgs }} -P "$MAVEN_PROFILES" | ||
| mvn -B dependency:go-offline clean package -DskipTests=true $EXTRA_MAVEN_ARGS -P "$MAVEN_PROFILES" | ||
|
|
||
| - name: Build and Package (dry-run) | ||
| if: ${{ inputs.dry_run == true }} | ||
| @@ -179,9 +177,10 @@ | ||
| env: | ||
| MAVEN_PROFILES: ${{ inputs.mavenProfiles }} | ||
| DRY_RUN_VERSION: ${{ inputs.dry_run_version }} | ||
| EXTRA_MAVEN_ARGS: ${{ inputs.extraMavenArgs }} | ||
| run: | | ||
| mvn -B release:clean release:prepare -DdryRun=true -Darguments="-Dmaven.javadoc.skip=true -Dmaven.test.skipTests=true -Dmaven.test.skip=true -Dmaven.deploy.skip=true" -DcheckModificationExcludeList=** -DignoreSnapshots=true -DreleaseVersion=$DRY_RUN_VERSION -DpushChanges=false -P "$MAVEN_PROFILES" -DscmServerId=liquibase | ||
| mvn -B dependency:go-offline clean package -DskipTests=true ${{ inputs.extraMavenArgs }} -P "$MAVEN_PROFILES" | ||
| mvn -B dependency:go-offline clean package -DskipTests=true $EXTRA_MAVEN_ARGS -P "$MAVEN_PROFILES" | ||
|
|
||
| - name: Get Artifact ID | ||
| working-directory: ${{ inputs.artifactPath }} |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.github/workflows/extension-attach-artifact-release.yml (1)
454-456: Branch detection in detached HEAD state will fail (unfixed from previous review).Line 454 uses
git rev-parse --abbrev-ref HEAD, which returns"HEAD"in GitHub Actions' detached checkout state rather than the actual branch name. This was flagged in the previous review but remains unresolved. Use GitHub's built-in ref variables instead:CURRENT_BRANCH="${{ github.head_ref || github.ref_name }}"This correctly resolves the PR source branch (for PRs) or current branch/tag (for other contexts), consistent with the default-branch approach used for normal releases (line 428).
Apply this diff:
- CURRENT_BRANCH=$(git rev-parse --abbrev-ref HEAD) + CURRENT_BRANCH="${{ github.head_ref || github.ref_name }}"
🧹 Nitpick comments (1)
.github/workflows/extension-attach-artifact-release.yml (1)
574-581: Quote path variable infindcommand to prevent word splitting and glob expansion.Line 579 uses the artifact path output in a
findcommand without proper quoting. IfARTIFACT_PATHcontains spaces, wildcards, or other special characters, the command could fail or behave unexpectedly. Wrap the path variable in quotes:- find ${{ steps.set-artifact-path.outputs.ARTIFACT_PATH }} -type f \( -name "*.jar" -o -name "*.pom" -o -name "*.asc" \) -path "*/target/*" ! -name "original-*" -exec cp {} /tmp/release-artifacts/ \; + find "${{ steps.set-artifact-path.outputs.ARTIFACT_PATH }}" -type f \( -name "*.jar" -o -name "*.pom" -o -name "*.asc" \) -path "*/target/*" ! -name "original-*" -exec cp {} /tmp/release-artifacts/ \;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
.github/workflows/extension-attach-artifact-release.yml(7 hunks)
🧰 Additional context used
🪛 GitHub Check: CodeQL
.github/workflows/extension-attach-artifact-release.yml
[warning] 170-170: Code injection
Potential code injection in ${{ github.event.inputs.liquibaseVersion }}, which may be controlled by an external user.
[warning] 174-174: Code injection
Potential code injection in ${{ inputs.extraMavenArgs }}, which may be controlled by an external user.
[warning] 182-182: Code injection
Potential code injection in ${{ inputs.dry_run_version }}, which may be controlled by an external user.
[warning] 183-183: Code injection
Potential code injection in ${{ inputs.extraMavenArgs }}, which may be controlled by an external user.
[warning] 203-203: Code injection
Potential code injection in ${{ inputs.dry_run }}, which may be controlled by an external user.
[warning] 204-204: Code injection
Potential code injection in ${{ inputs.dry_run_version }}, which may be controlled by an external user.
[warning] 424-424: Code injection
Potential code injection in ${{ github.event.inputs.liquibaseVersion }}, which may be controlled by an external user.
[warning] 424-424: Code injection
Potential code injection in ${{ inputs.mavenProfiles }}, which may be controlled by an external user.
[warning] 428-428: Code injection
Potential code injection in ${{ github.event.repository.default_branch }}, which may be controlled by an external user.
[warning] 430-430: Code injection
Potential code injection in ${{ inputs.mavenProfiles }}, which may be controlled by an external user.
[warning] 444-444: Code injection
Potential code injection in ${{ inputs.dry_run_version }}, which may be controlled by an external user.
[warning] 451-451: Code injection
Potential code injection in ${{ inputs.dry_run_version }}, which may be controlled by an external user.
[warning] 451-451: Code injection
Potential code injection in ${{ inputs.mavenProfiles }}, which may be controlled by an external user.
[warning] 456-456: Code injection
Potential code injection in ${{ inputs.mavenProfiles }}, which may be controlled by an external user.
[warning] 572-572: Code injection
Potential code injection in ${{ inputs.artifactPath || '.' }}, which may be controlled by an external user.
[warning] 579-579: Code injection
Potential code injection in ${{ steps.set-artifact-path.outputs.ARTIFACT_PATH }}, which may be controlled by an external user.
🔇 Additional comments (4)
.github/workflows/extension-attach-artifact-release.yml (4)
203-208: Approve unified artifact version resolution.The unified "Get Artifact Version" step correctly branches on
dry_runto use either the explicitdry_run_versioninput or dynamically evaluateproject.versionfrom the POM. This aligns with the PR objectives and simplifies version management.
423-439: Approve normal release artifact generation path.Using
github.event.repository.default_branch(line 428) is a solid improvement for branch detection. The artifact generation—source/javadoc JARs, versioned POM—is comprehensive and well-structured. Version evaluation via Maven (line 437) ensures consistency with actual build output.
442-465: Verify version handling consistency betweenversions:setandrelease:preparefor dry-runs.For dry-runs, the workflow sets the POM version to
dry_run_version-SNAPSHOT(line 444) before runningrelease:preparewith-DdryRun=true(lines 450–452). This differs from the normal release path, which directly invokesrelease:preparewith-DreleaseVersion.The order of operations (versions:set → release:prepare → source:jar/javadoc:jar) should produce artifacts with the SNAPSHOT version, which are then signed with the dynamically evaluated
project.version(line 542). Verify that:
- After
versions:setandrelease:prepare, generated JARs are named with the SNAPSHOT version.- The
project.versionevaluated at line 542 matches the generated artifact filenames.- No conflicts arise between Maven Release Plugin's version management and
versions:set.
542-547: Approve dry-run signing with improved version logging.Adding version logging (line 543) enhances observability for debugging. The dynamic
project.versionevaluation ensures the signing references the same version used for artifact naming after the POM modification at line 444. This maintains consistency between generated artifacts and their signatures.
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
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 (1)
.github/workflows/extension-attach-artifact-release.yml (1)
604-611:⚠️ Zip attachment logic does not account for dry-run artifact relocation.The zip attachment condition (line 605) only checks
inputs.zip == 'true'but does not exclude dry-run mode. For dry-run workflows, artifacts are collected to/tmp/release-artifacts(line 580), butASSET_DIRis hardcoded to./target(line 611). This mismatch will cause the upload to fail or attach incorrect/missing files during a dry-run withzip: true.Fix: Either add a dry-run exclusion to the condition, or make
ASSET_DIRconditional:- name: Attach Zip File to Draft Release if: inputs.zip == 'true' && inputs.dry_run == false id: upload-release-zip run: ./.github/upload_zip.sh $(mvn help:evaluate -Dexpression=project.version -q -DforceStdout) env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} ASSET_NAME_PREFIX: "${{ env.artifact_id }}-" ASSET_DIR: ./targetAlternatively, if zip should work for dry-run, pass the correct directory:
- name: Attach Zip File to Draft Release if: inputs.zip == 'true' id: upload-release-zip run: ./.github/upload_zip.sh $(mvn help:evaluate -Dexpression=project.version -q -DforceStdout) env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} ASSET_NAME_PREFIX: "${{ env.artifact_id }}-" ASSET_DIR: ${{ inputs.dry_run == true && '/tmp/release-artifacts' || './target' }}
🧹 Nitpick comments (1)
.github/workflows/extension-attach-artifact-release.yml (1)
453-468: Dry-run build properly mirrors normal release with branch detection improved.The changes correctly apply
-DdryRun=true, use current branch forbuildNumber, and generate source/javadoc/POM artifacts. The branch detection shift fromgit rev-parseto GitHub context variables addresses detached HEAD concerns.However, the actionlint warning at line 457 suggests passing
github.head_refandgithub.ref_namethrough an environment variable for stricter security isolation:env: CURRENT_BRANCH: ${{ github.head_ref || github.ref_name }} run: | echo "Using current branch: $CURRENT_BRANCH" mvn clean install -DskipTests -P '${{ inputs.mavenProfiles }}' -DbuildNumber=$CURRENT_BRANCHThis is a minor security hygiene improvement.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
.github/workflows/extension-attach-artifact-release.yml(7 hunks)
🧰 Additional context used
🪛 actionlint (1.7.8)
.github/workflows/extension-attach-artifact-release.yml
452-452: "github.head_ref" is potentially untrusted. avoid using it directly in inline scripts. instead, pass it through an environment variable. see https://docs.github.com/en/actions/reference/security/secure-use#good-practices-for-mitigating-script-injection-attacks for more details
(expression)
🪛 GitHub Check: CodeQL
.github/workflows/extension-attach-artifact-release.yml
[warning] 170-170: Code injection
Potential code injection in ${{ github.event.inputs.liquibaseVersion }}, which may be controlled by an external user.
[warning] 174-174: Code injection
Potential code injection in ${{ inputs.extraMavenArgs }}, which may be controlled by an external user.
[warning] 184-184: Code injection
Potential code injection in ${{ inputs.extraMavenArgs }}, which may be controlled by an external user.
[warning] 204-204: Code injection
Potential code injection in ${{ inputs.dry_run }}, which may be controlled by an external user.
[warning] 205-205: Code injection
Potential code injection in ${{ inputs.dry_run_version }}, which may be controlled by an external user.
[warning] 425-425: Code injection
Potential code injection in ${{ github.event.inputs.liquibaseVersion }}, which may be controlled by an external user.
[warning] 425-425: Code injection
Potential code injection in ${{ inputs.mavenProfiles }}, which may be controlled by an external user.
[warning] 429-429: Code injection
Potential code injection in ${{ github.event.repository.default_branch }}, which may be controlled by an external user.
[warning] 431-431: Code injection
Potential code injection in ${{ inputs.mavenProfiles }}, which may be controlled by an external user.
[warning] 454-454: Code injection
Potential code injection in ${{ inputs.dry_run_version }}, which may be controlled by an external user.
[warning] 454-454: Code injection
Potential code injection in ${{ inputs.mavenProfiles }}, which may be controlled by an external user.
[warning] 457-457: Code injection
Potential code injection in ${{ github.head_ref || github.ref_name }}, which may be controlled by an external user.
[warning] 459-459: Code injection
Potential code injection in ${{ inputs.mavenProfiles }}, which may be controlled by an external user.
[warning] 575-575: Code injection
Potential code injection in ${{ inputs.artifactPath || '.' }}, which may be controlled by an external user.
[warning] 582-582: Code injection
Potential code injection in ${{ steps.set-artifact-path.outputs.ARTIFACT_PATH }}, which may be controlled by an external user.
🔇 Additional comments (6)
.github/workflows/extension-attach-artifact-release.yml (6)
169-184: Maven environment variables and dry-run flag properly added.The use of
MAVEN_PROFILESenvironment variable (lines 170, 180) and-DdryRun=true(line 183) follow security best practices and are correctly implemented. The duplication between normal and dry-run steps is acceptable given workflow readability.
204-209: Artifact version resolution properly consolidated.The unified "Get Artifact Version" step correctly handles both dry-run and normal release paths. The conditional at line 204 properly selects between
dry_run_versioninput and evaluatedproject.version.
424-440: Release artifact generation properly enhanced with source/javadoc and versioned POM.Lines 424–440 correctly implement versioned POM copying, source/javadoc JAR generation, and repository default-branch usage. The pattern matches the dry-run flow for consistency across release types.
442-447: POM version setting for dry-run correctly isolates version modification.The dedicated step (lines 442–447) properly sets POM version to
${DRY_RUN_VERSION}-SNAPSHOTbefore the dry-run build, ensuring artifacts are versioned consistently and distinguishably for test/simulation purposes.
534-550: Signing logic correctly uses evaluated project.version for both release types.Both normal (line 534) and dry-run (line 545) signing steps now extract
project.versionfrom Maven rather than using the input parameter. Line 546 adds helpful logging for dry-run debugging. This ensures signed artifacts match the generated files' naming.
572-596: Dry-run artifact collection and attachment properly centralized.The new steps (572–596) correctly isolate dry-run artifacts in
/tmp/release-artifacts, support multi-module projects via recursive find, and usesoftprops/action-gh-releasefor draft release creation. The artifact filtering (excludingoriginal-*) is appropriate.
| working-directory: ${{ inputs.artifactPath }} | ||
| run: | | ||
| # Validate and sanitize dry_run_version input (Maven-style: letters, digits, dots, hyphens, underscores) | ||
| raw_version="${{ inputs.dry_run_version }}" |
Check warning
Code scanning / CodeQL
Code injection Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 2 days ago
To fix the issue, we must not interpolate ${{ inputs.dry_run_version }} directly into shell code. Instead, we should follow the GitHub Actions best practice:
- Set the untrusted input as an environment variable using the workflow’s
env:block. - In the shell script, reference that value only via the native shell environment variable syntax (
$VAR), never as${{ env.VAR }}or via another workflow expression. - Ensure the shell variable is quoted for further safety.
How to implement:
- In the
- name: Rename SNAPSHOTS to dry_run versionstep, add anenv:section that setsRAW_VERSION: ${{ inputs.dry_run_version }}before therun:block. - In the script itself, replace
raw_version="${{ inputs.dry_run_version }}"withraw_version="$RAW_VERSION". - All validation and subsequent usage remain unchanged.
This approach avoids direct user input interpolation into the script and hardens the workflow against possible future misuse.
| @@ -329,9 +329,11 @@ | ||
|
|
||
| - name: Rename SNAPSHOTS to dry_run version | ||
| working-directory: ${{ inputs.artifactPath }} | ||
| env: | ||
| RAW_VERSION: ${{ inputs.dry_run_version }} | ||
| run: | | ||
| # Validate and sanitize dry_run_version input (Maven-style: letters, digits, dots, hyphens, underscores) | ||
| raw_version="${{ inputs.dry_run_version }}" | ||
| raw_version="$RAW_VERSION" | ||
| if [[ ! "$raw_version" =~ ^[a-zA-Z0-9._-]+$ ]]; then | ||
| echo "ERROR: Invalid dry_run_version format: '$raw_version'" | ||
| echo "Version must contain only letters, digits, dots, hyphens, and underscores" |
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
| DRY_RUN_VERSION: ${{ inputs.dry_run_version }} | ||
| run: | | ||
| artifact_version=$(mvn help:evaluate "-Dexpression=project.version" -q -DforceStdout) | ||
| if [ "${{ inputs.dry_run }}" == "true" ]; then |
Check warning
Code scanning / CodeQL
Code injection Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 2 days ago
The best way to fix this issue is to avoid directly interpolating ${{ inputs.dry_run }} into the shell script. Instead, set an intermediate environment variable—DRY_RUN—whose value comes from ${{ inputs.dry_run }} and reference it only using shell variable syntax ($DRY_RUN) within the shell script. This avoids injection risk by ensuring that the shell treats the input as a variable value and not as executable code. The fix should be made in the step under lines 205-212 (run: block of "Get Artifact Version"). Add DRY_RUN: ${{ inputs.dry_run }} under the env: section, and replace ${{ inputs.dry_run }} with $DRY_RUN in the shell script.
No additional imports or definitions are needed. The required code changes are limited to the workflow file: adding the env variable and referencing it safely.
| @@ -202,8 +202,9 @@ | ||
| shell: bash | ||
| env: | ||
| DRY_RUN_VERSION: ${{ inputs.dry_run_version }} | ||
| DRY_RUN: ${{ inputs.dry_run }} | ||
| run: | | ||
| if [ "${{ inputs.dry_run }}" == "true" ]; then | ||
| if [ "$DRY_RUN" == "true" ]; then | ||
| artifact_version="$DRY_RUN_VERSION" | ||
| else | ||
| artifact_version=$(mvn help:evaluate "-Dexpression=project.version" -q -DforceStdout) |
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
This pull request updates the release workflow for extension artifacts, improving how dry runs and actual releases are handled in the GitHub Actions workflows. The changes streamline artifact versioning, improve branch handling, enhance artifact generation, and ensure consistent artifact naming for both dry runs and releases.
Dry Run and Release Process Improvements
-DdryRun=trueto Maven release commands for dry runs, ensuring no actual changes are pushed during dry runs. [1] [2]-SNAPSHOTsuffix, and later renamed files and updated POM contents to remove-SNAPSHOTfor consistency. [1] [2]Branch and Build Metadata Handling
masterormainbranches to using GitHub's default branch in release builds, and using the current branch for dry runs. This improves reliability and matches repository settings.Artifact Generation and Signing
Artifact Path and Attachment
Snapshot Repository Enablement
These changes collectively improve the reliability, clarity, and consistency of the extension artifact release workflows.