[MOS-29972]Updated Sonar analysis workflow configuration - #336
Conversation
Signed-off-by: Mahesh-Binayak <76687012+Mahesh-Binayak@users.noreply.github.com>
WalkthroughThe workflow file is refactored to add explicit input defaults, restructure job setup steps, simplify Maven security configuration, and rewrite SonarCloud analysis scripting with aggressive service key normalization and dynamic project key composition from repository name and normalized service identifiers. ChangesSonarCloud Analysis Workflow
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
.github/workflows/maven-sonar-analysis-new.yml (3)
35-43: ⚡ Quick winUpdate actions to v4 and set
persist-credentials: false.Static analysis indicates
actions/checkout@v3andactions/setup-java@v3are outdated. Both have v4 releases with security improvements. Additionally, settingpersist-credentials: falseon checkout prevents unnecessary credential persistence in the git config.🔧 Suggested updates
- - uses: actions/checkout@v3 + - uses: actions/checkout@v4 + with: + persist-credentials: false - name: Set up JDK 21 - uses: actions/setup-java@v3 + uses: actions/setup-java@v4 with:🤖 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/maven-sonar-analysis-new.yml around lines 35 - 43, Update the GitHub Actions steps to use the v4 releases and disable credential persistence: change uses: actions/checkout@v3 to uses: actions/checkout@v4 and add persist-credentials: false to that checkout step; change uses: actions/setup-java@v3 to uses: actions/setup-java@v4 while keeping existing inputs (distribution: temurin, java-version: '21', server-id, settings-path) intact so behavior remains the same.
72-72: ⚡ Quick winTemplate injection:
inputs.SERVICE_LOCATIONused directly in shell.The raw input value is used in the
cdcommand. If a caller passes a value containing$(...)or backticks, command substitution would occur. While callers are expected to be trusted in workflow_call contexts, using an environment variable with proper quoting is defensive.🔧 Safer pattern
- name: Sonar Analysis + env: + SERVICE_LOCATION: ${{ inputs.SERVICE_LOCATION || '.' }} + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} run: | - SERVICE_KEY=$(echo "${{ inputs.SERVICE_LOCATION }}" | sed 's|^\./||; s|^\.$||; s|[^a-zA-Z0-9]|-|g; s|-\\+|-|g; s|^-||; s|-$||') + SERVICE_KEY=$(echo "$SERVICE_LOCATION" | sed 's|^\./||; s|^\.$||; s|[^a-zA-Z0-9]|-|g; s|-\+|-|g; s|^-||; s|-$||') FINAL_NAME="${{ github.event.repository.name }}${SERVICE_KEY:+-$SERVICE_KEY}" FULL_PROJECT_KEY="${{ secrets.ORG_KEY }}_${FINAL_NAME}" - cd "${{ inputs.SERVICE_LOCATION || '.' }}" && \ + cd "$SERVICE_LOCATION" && \ mvn -U -B verify sonar:sonar \Moving the input to an environment variable prevents template injection since the value isn't interpolated into the shell script itself.
🤖 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/maven-sonar-analysis-new.yml at line 72, The workflow currently interpolates inputs.SERVICE_LOCATION directly into the shell command (cd "${{ inputs.SERVICE_LOCATION || '.' }}" && \) allowing potential shell/template injection; instead set the input into an environment variable (e.g., SERVICE_LOCATION) via env: and then use the env var in the shell step (cd "$SERVICE_LOCATION" || cd '.') with proper double quotes so the value is not template-expanded into the script; update the step that references inputs.SERVICE_LOCATION to read from the environment variable and ensure the fallback to '.' is handled in the shell using a safe quoted expression.
52-55: ⚡ Quick winAvoid direct template interpolation to mitigate injection risk.
${{ github.ref }}is interpolated directly into the shell command. While the risk is lower in aworkflow_callcontext, using an environment variable is safer and avoids potential command injection if a ref contains shell metacharacters.🔧 Safer pattern using env
- name: Setup env - run: | - echo "BRANCH_NAME=$(echo ${{ github.ref }} | sed -e 's,.*/\(.*\),\1,')" >> $GITHUB_ENV + run: | + echo "BRANCH_NAME=$(echo "$GITHUB_REF" | sed -e 's,.*/\(.*\),\1,')" >> $GITHUB_ENV echo "GPG_TTY=$(tty)" >> $GITHUB_ENV
GITHUB_REFis a built-in environment variable that's safer to use than template expansion.🤖 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/maven-sonar-analysis-new.yml around lines 52 - 55, In the "Setup env" step avoid template interpolation of github.ref; instead read the built-in environment variable GITHUB_REF inside the shell and use that to compute BRANCH_NAME. Update the command that sets BRANCH_NAME (referencing the BRANCH_NAME assignment in the "Setup env" step) to use $GITHUB_REF rather than ${{ github.ref }}, and keep the GPG_TTY assignment (GPG_TTY and tty usage) unchanged.
🤖 Prompt for all review comments with 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.
Inline comments:
In @.github/workflows/maven-sonar-analysis-new.yml:
- Line 69: The SERVICE_KEY sed pipeline uses the literal sequence s|-\\+|-|g
which matches a backslash plus instead of collapsing consecutive hyphens; update
the sed substitution in the SERVICE_KEY assignment to use a proper regex such as
s|-\+|-|g for GNU sed or s|--*|-|g for POSIX compatibility so multiple
consecutive '-' are collapsed into a single '-' (replace the s|-\\+|-|g token
accordingly).
---
Nitpick comments:
In @.github/workflows/maven-sonar-analysis-new.yml:
- Around line 35-43: Update the GitHub Actions steps to use the v4 releases and
disable credential persistence: change uses: actions/checkout@v3 to uses:
actions/checkout@v4 and add persist-credentials: false to that checkout step;
change uses: actions/setup-java@v3 to uses: actions/setup-java@v4 while keeping
existing inputs (distribution: temurin, java-version: '21', server-id,
settings-path) intact so behavior remains the same.
- Line 72: The workflow currently interpolates inputs.SERVICE_LOCATION directly
into the shell command (cd "${{ inputs.SERVICE_LOCATION || '.' }}" && \)
allowing potential shell/template injection; instead set the input into an
environment variable (e.g., SERVICE_LOCATION) via env: and then use the env var
in the shell step (cd "$SERVICE_LOCATION" || cd '.') with proper double quotes
so the value is not template-expanded into the script; update the step that
references inputs.SERVICE_LOCATION to read from the environment variable and
ensure the fallback to '.' is handled in the shell using a safe quoted
expression.
- Around line 52-55: In the "Setup env" step avoid template interpolation of
github.ref; instead read the built-in environment variable GITHUB_REF inside the
shell and use that to compute BRANCH_NAME. Update the command that sets
BRANCH_NAME (referencing the BRANCH_NAME assignment in the "Setup env" step) to
use $GITHUB_REF rather than ${{ github.ref }}, and keep the GPG_TTY assignment
(GPG_TTY and tty usage) unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 72d0d10f-36dd-445c-8d0d-7673d15bf1c2
📒 Files selected for processing (1)
.github/workflows/maven-sonar-analysis-new.yml
Summary by CodeRabbit