Improve CI gates: drift checks, coverage, CodeQL, PR analysis - #6
Improve CI gates: drift checks, coverage, CodeQL, PR analysis#6catalintomai wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughThe pull request replaces the shared CI workflow with explicit validation, coverage, delivery, and release jobs. It also adds CodeQL configuration and workflows for automated pull request test analysis and authorized overrides. ChangesCI pipeline
Analysis automation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant PullRequest
participant GitHubActions
participant MattermostCI
participant Postgres14
participant ArtifactStorage
PullRequest->>GitHubActions: trigger CI workflow
GitHubActions->>MattermostCI: run lint, test, and build jobs
MattermostCI->>Postgres14: execute database-backed tests
GitHubActions->>ArtifactStorage: deliver or release renamed artifacts
sequenceDiagram
participant PullRequest
participant GitHubActions
participant TestAutomationToolkit
PullRequest->>GitHubActions: trigger PR test analysis
GitHubActions->>TestAutomationToolkit: delegate analysis with PR context
PullRequest->>GitHubActions: submit authorized override comment
GitHubActions->>TestAutomationToolkit: delegate override workflow
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)
16-214: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winNo
timeout-minutesset on any job.All six jobs (
lint,test,build,coverage,delivery,release) rely on the GitHub-wide default (360 minutes) rather than an explicit bound. A hung DB connection, network call, or external action could tie up a runner for hours instead of failing fast.♻️ Example
lint: if: ${{ github.repository_owner == 'mattermost' || github.event_name != 'schedule' }} runs-on: ubuntu-latest + timeout-minutes: 15 permissions: contents: read🤖 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/ci.yml around lines 16 - 214, Add explicit timeout-minutes values to all six workflow jobs: lint, test, build, coverage, delivery, and release. Choose reasonable bounds that allow normal database, build, coverage, artifact, and upload operations while ensuring hung commands or external actions fail promptly instead of using GitHub’s 360-minute default.
🤖 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/codeql/codeql-config.yml:
- Around line 3-9: Remove the global go/log-injection exclusion from the
query-filters in the CodeQL configuration. Keep the severity exclusions
unchanged, and do not replace the removed rule exclusion with another
repository-wide suppression; scope any necessary exception to the specific
finding only.
In @.github/workflows/codeql-analysis.yml:
- Around line 27-28: Update the actions/checkout step in the CodeQL workflow to
disable credential persistence by setting its persist-credentials option to
false, while preserving the pinned action version and existing checkout
behavior.
- Around line 16-18: Update the analyze job’s permissions block to include
contents: read alongside security-events: write, ensuring actions/checkout
retains repository read access while preserving CodeQL’s security-events
permission.
In @.github/workflows/pr-test-analysis.yml:
- Line 38: Pin all referenced reusable workflows and CodeQL actions to immutable
commit SHAs instead of the mutable main and v3.28.9 references: update
.github/workflows/pr-test-analysis.yml lines 38-38,
.github/workflows/codeql-analysis.yml lines 31-41 for init, autobuild, and
analyze, and .github/workflows/pr-test-analysis-override.yml lines 23-23.
Preserve the existing workflow and action versions while replacing each tag or
branch with its corresponding verified commit SHA.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 16-214: Add explicit timeout-minutes values to all six workflow
jobs: lint, test, build, coverage, delivery, and release. Choose reasonable
bounds that allow normal database, build, coverage, artifact, and upload
operations while ensuring hung commands or external actions fail promptly
instead of using GitHub’s 360-minute default.
🪄 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: 53a174d6-7a78-45b0-a8d5-295e6bc79c20
📒 Files selected for processing (5)
.github/codeql/codeql-config.yml.github/workflows/ci.yml.github/workflows/codeql-analysis.yml.github/workflows/pr-test-analysis-override.yml.github/workflows/pr-test-analysis.yml
| query-filters: | ||
| - exclude: | ||
| problem.severity: | ||
| - warning | ||
| - recommendation | ||
| - exclude: | ||
| id: go/log-injection |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Top-level files:\n'
git ls-files | sed -n '1,120p'
printf '\nCodeQL config:\n'
cat -n .github/codeql/codeql-config.yml
printf '\nGo files in repository:\n'
git ls-files '*.go' | sed -n '1,200p'
printf '\nSearch for log-related sinks / suppressions:\n'
rg -n --hidden --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**' 'go/log-injection|codeql|log-injection|logger|log\.' .Repository: mattermost/mattermost-plugin-docs
Length of output: 2623
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Outline of Go files with logging-related names:\n'
ast-grep outline build/pluginctl/logs.go --view expanded || true
ast-grep outline server/main.go --view expanded || true
ast-grep outline server/plugin.go --view expanded || true
ast-grep outline server/api.go --view expanded || true
ast-grep outline server/job.go --view expanded || true
printf '\nLogging call sites:\n'
rg -n 'log\.|Printf|Println|Errorf|Infof|Warnf|logger|zap|slog' build/pluginctl server -g '*.go'
printf '\nRelevant snippets:\n'
sed -n '1,220p' build/pluginctl/logs.go
printf '\n--- server/main.go ---\n'
sed -n '1,220p' server/main.go
printf '\n--- server/plugin.go ---\n'
sed -n '1,260p' server/plugin.go
printf '\n--- server/api.go ---\n'
sed -n '1,240p' server/api.go
printf '\n--- server/job.go ---\n'
sed -n '1,240p' server/job.goRepository: mattermost/mattermost-plugin-docs
Length of output: 13249
Remove the global go/log-injection exclusion in .github/codeql/codeql-config.yml:3-9. This disables CodeQL’s log-injection check across every Go file in the repo; keep it only with a documented repository-wide false-positive review, otherwise scope it to the specific finding.
🤖 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/codeql/codeql-config.yml around lines 3 - 9, Remove the global
go/log-injection exclusion from the query-filters in the CodeQL configuration.
Keep the severity exclusions unchanged, and do not replace the removed rule
exclusion with another repository-wide suppression; scope any necessary
exception to the specific finding only.
| permissions: | ||
| security-events: write | ||
| name: Analyze |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '== .github/workflows/codeql-analysis.yml (outline) ==\n'
ast-grep outline .github/workflows/codeql-analysis.yml --view expanded || true
printf '\n== .github/workflows/codeql-analysis.yml (numbered, first 240 lines) ==\n'
cat -n .github/workflows/codeql-analysis.yml | sed -n '1,240p'
printf '\n== Search for permissions blocks in workflow files ==\n'
rg -n --glob '.github/workflows/*.yml' --glob '.github/workflows/*.yaml' 'permissions:|contents: read|security-events: write|actions/checkout|github/codeql-action' .github/workflowsRepository: mattermost/mattermost-plugin-docs
Length of output: 3583
🌐 Web query:
GitHub Actions job-level permissions override workflow-level permissions unspecified set to none contents read actions/checkout
💡 Result:
In GitHub Actions, when you define the permissions key at the job level, it completely overrides any permissions defined at the workflow level [1][2]. The permissions do not merge; the job-level configuration becomes the definitive set of permissions for that specific job [2]. When you explicitly use the permissions key—at either the workflow or job level—any permissions that you do not specify are automatically set to none (no access), with the sole exception of the metadata scope, which always retains read access [1][3][4]. For the actions/checkout action, the recommended configuration to ensure it can successfully check out your repository is to explicitly grant read access to the contents scope [5][6][7]: permissions: contents: read If you have a workflow-level permissions block but need to override it for a specific job while maintaining the actions/checkout functionality, you must include contents: read in that job's permissions block, as it will otherwise be reset to none [1][2].
Citations:
- 1: https://docs.github.com/en/actions/reference/workflows-and-actions/workflow-syntax
- 2: https://adaptive-enforcement-lab.com/secure/github-actions-security/token-permissions/job-scoping/
- 3: https://dev.to/github/the-githubtoken-in-github-actions-how-it-works-change-permissions-customizations-3cgp
- 4: "Modifying the permissions for the
GITHUB_TOKEN" should use a callout box for "When thepermissionskey is used, all unspecified permissions are set to no access" github/docs#35810 - 5: https://github.com/actions/checkout/?tab=readme-ov-file
- 6: https://github.com/actions/checkout?tab=readme-ov-file
- 7: https://github.com/actions/checkout/blob/main/README.md
Add contents: read to the job permissions.
jobs.analyze.permissions replaces the workflow-level block, so actions/checkout loses repo read access here and can fail before CodeQL starts.
🤖 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/codeql-analysis.yml around lines 16 - 18, Update the
analyze job’s permissions block to include contents: read alongside
security-events: write, ensuring actions/checkout retains repository read access
while preserving CodeQL’s security-events permission.
| - name: Checkout repository | ||
| uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Disable checkout credential persistence.
actions/checkout stores GITHUB_TOKEN in the local Git configuration by default. Later build or analysis tooling could expose that credential through an artefact.
- name: Checkout repository
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
+ with:
+ persist-credentials: false📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Checkout repository | |
| uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 | |
| - name: Checkout repository | |
| uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 | |
| with: | |
| persist-credentials: false |
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 27-28: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
🤖 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/codeql-analysis.yml around lines 27 - 28, Update the
actions/checkout step in the CodeQL workflow to disable credential persistence
by setting its persist-credentials option to false, while preserving the pinned
action version and existing checkout behavior.
Source: Linters/SAST tools
| github.event_name == 'workflow_dispatch' || | ||
| (github.event.pull_request.draft == false && | ||
| github.event.pull_request.head.repo.full_name == 'mattermost/mattermost-plugin-docs') | ||
| uses: mattermost/mattermost-test-automation-toolkit/.github/workflows/pr-test-analysis.yml@main |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
for f in .github/workflows/pr-test-analysis.yml .github/workflows/codeql-analysis.yml .github/workflows/pr-test-analysis-override.yml; do
echo "===== $f ====="
nl -ba "$f" | sed -n '1,120p'
echo
doneRepository: mattermost/mattermost-plugin-docs
Length of output: 262
🏁 Script executed:
#!/bin/bash
set -euo pipefail
for f in .github/workflows/pr-test-analysis.yml .github/workflows/codeql-analysis.yml .github/workflows/pr-test-analysis-override.yml; do
echo "===== $f ====="
sed -n '1,140p' "$f"
echo
doneRepository: mattermost/mattermost-plugin-docs
Length of output: 3976
Pin reusable workflows and CodeQL actions to commit SHAs.
These references still use mutable tags/branches while running with repository permissions and secrets:
.github/workflows/pr-test-analysis.yml:mattermost/mattermost-test-automation-toolkit/.github/workflows/pr-test-analysis.yml@main.github/workflows/codeql-analysis.yml:github/codeql-action/{init,autobuild,analyze}@v3.28.9``.github/workflows/pr-test-analysis-override.yml:mattermost/mattermost-test-automation-toolkit/.github/workflows/pr-test-analysis-override.yml@main
📍 Affects 3 files
.github/workflows/pr-test-analysis.yml#L38-L38(this comment).github/workflows/codeql-analysis.yml#L31-L41.github/workflows/pr-test-analysis-override.yml#L23-L23
🤖 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/pr-test-analysis.yml at line 38, Pin all referenced
reusable workflows and CodeQL actions to immutable commit SHAs instead of the
mutable main and v3.28.9 references: update
.github/workflows/pr-test-analysis.yml lines 38-38,
.github/workflows/codeql-analysis.yml lines 31-41 for init, autobuild, and
analyze, and .github/workflows/pr-test-analysis-override.yml lines 23-23.
Preserve the existing workflow and action versions while replacing each tag or
branch with its corresponding verified commit SHA.
1 participant
Summary
ci.ymlfrom a single delegated workflow call into explicit jobs:lint,test,build,coverage,delivery, andreleasego.mod, plugin manifest, and i18n files in the lint jobcoveragejob that runs Go tests with a real Postgres and uploads results to Codecov (requiresCODECOV_TOKENrepo secret).github/workflows/codeql-analysis.yml) for Go and JavaScript security scanningpr-test-analysis.yml/pr-test-analysis-override.yml) matching the pattern used by sibling plugins (calls, agents)Notes
coveragejob uses${{ secrets.CODECOV_TOKEN }}— aCODECOV_TOKENrepository secret must be configured in repo settings for uploads to work