-
Notifications
You must be signed in to change notification settings - Fork 82
[PM-28029] Address SonarCloud and zizmor errors #2124
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Claude finished @vvolkgang's task —— View job Review Complete
SummaryThis PR addresses security findings from SonarCloud and zizmor by removing an unused GitHub Action, eliminating unnecessary secret inheritance in version calculation jobs, and fixing a bash parameter expansion bug. The changes are minimal and security-focused. FindingsFinding 1: The 💭 These descriptions should be updated or the parameter should be evaluated for removal if no longer needed. Suggested fix# Either update the description:
distinct_id:
description: "Unique ID for workflow dispatch tracking"
# Or if truly unused, consider removing the parameter entirelyFinding 2: The PR only removes
ContextThe Question for the author: Did the zizmor/SonarCloud finding specifically flag only the Finding 3: The bash parameter expansion fix at .github/workflows/_version.yml:111 correctly adds a default value ( 👍 Good defensive programming - this handles the edge case properly. PR Title and Description Quality💭 The PR description is clear and concise. However, it could be improved by:
Suggested additions to PR description## Security Context
The `_version.yml` workflow performs only version calculations using git operations and doesn't require access to repository secrets. The zizmor tool flagged this as a violation of the principle of least privilege - secrets should only be inherited by jobs that actually need them.
The build jobs (`build-manual` and `build-public`) still use `secrets: inherit` as they legitimately require secrets for code signing and distribution.Good Practices Observed
Action Items
|
|
Great job! No new security vulnerabilities introduced in this pull request |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2124 +/- ##
==========================================
- Coverage 85.38% 84.04% -1.35%
==========================================
Files 1726 1982 +256
Lines 145621 161180 +15559
==========================================
+ Hits 124343 135459 +11116
- Misses 21278 25721 +4443 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
# Conflicts: # .github/actions/dispatch-and-download/action.yml
| latest_tag_version=$(git tag -l --sort=-creatordate | grep "$APP_CODENAME" | head -n 1) | ||
| if [[ -z "$latest_tag_version" ]]; then | ||
| version_name="${current_year}.${current_month}.${_PATCH_VERSION}" | ||
| version_name="${current_year}.${current_month}.${_PATCH_VERSION:-0}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Good fix - the :-0 default prevents bash errors when _PATCH_VERSION is unset and no tags exist in the repository.

🎟️ Tracking
PM-28029
📔 Objective
Address SonarCube and zizmor findings, following similar work in bitwarden/android#6151
dispatch-and-downloadaction.secrets: inheritfrom_version.ymljobs.⏰ Reminders before review
🦮 Reviewer guidelines
:+1:) or similar for great changes:memo:) or ℹ️ (:information_source:) for notes or general info:question:) for questions:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion:art:) for suggestions / improvements:x:) or:warning:) for more significant problems or concerns needing attention:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt:pick:) for minor or nitpick changes