ci: add Moodle Plugin CI workflow - #3
Conversation
WalkthroughSummary by CodeRabbit
WalkthroughA new GitHub Actions workflow file is added that defines a "Moodle Plugin CI" pipeline. It triggers on pushes to ChangesMoodle Plugin CI Workflow
|
There was a problem hiding this comment.
Actionable comments posted: 2
Nitpick comments (3)
.github/workflows/moodle-ci.yaml (3)
44-47: 💤 Low valueConsider setting
persist-credentials: falsefor defence in depth.The checkout step does not explicitly set
persist-credentials: false. Whilst this workflow does not perform git operations after checkout, setting this option prevents accidental credential leakage if the workflow is later modified.Proposed fix
- name: Check out repository code uses: actions/checkout@v4 with: path: plugin + persist-credentials: falsePrompt 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/moodle-ci.yaml around lines 44 - 47, The checkout action at the "Check out repository code" step does not explicitly set persist-credentials to false. Add persist-credentials: false to the with section of the actions/checkout@v4 action alongside the existing path: plugin configuration to prevent potential credential leakage if the workflow is modified in the future.Source: Linters/SAST tools
85-91: 💤 Low valueSimplify failure handling in PHPDoc step.
The PHPDoc checker step combines
|| exit 0(line 87) withcontinue-on-error: true(line 88). Both suppress failures, making one redundant. Using onlycontinue-on-error: trueis clearer and achieves the same result.Proposed fix
- name: Moodle PHPDoc Checker if: ${{ !cancelled() }} - run: moodle-plugin-ci phpdoc --max-warnings 0 || exit 0 + run: moodle-plugin-ci phpdoc --max-warnings 0 continue-on-error: true env: MDL_LOCAL_DEVKIT_DISABLE: 1Prompt 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/moodle-ci.yaml around lines 85 - 91, The "Moodle PHPDoc Checker" step has redundant failure handling with both `|| exit 0` in the run command and `continue-on-error: true` specified separately. Remove the `|| exit 0` from the moodle-plugin-ci phpdoc command to use only `continue-on-error: true`, which is the clearer and simpler approach for suppressing step failures.
45-45: ⚖️ Poor tradeoffConsider pinning GitHub Actions to commit SHAs for improved supply-chain security.
The workflow uses semantic version tags (
@v4,@v2) for actions at lines 45, 50, and 119. Whilst version tags are convenient and commonly used, pinning actions to specific commit SHAs provides stronger guarantees against tag tampering or malicious updates.Example of SHA pinning with a comment indicating the version:
# actions/checkout@v4.2.2 uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683Tools like Dependabot can help maintain SHA-pinned actions by automatically creating PRs when new versions are released.
Also applies to: 50-50, 119-119
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/moodle-ci.yaml at line 45, Replace the semantic version tags for GitHub Actions with specific commit SHAs to enhance supply chain security. At line 45, replace `actions/checkout@v4` with the full commit SHA format (e.g., `actions/checkout@<full-commit-hash>`) and add a comment above indicating the version (e.g., `# actions/checkout@v4.2.2`). Apply the same change pattern to the other affected locations at lines 50 and 119, replacing their version tags (`@v2` and others) with their corresponding commit SHAs and version comments. This prevents potential tag tampering and ensures pinned actions are immutable.Source: Linters/SAST tools
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/moodle-ci.yaml:
- Around line 117-125: The invalid GitHub Actions syntax `join(matrix.*, ', ')`
in the artifact upload step's name field uses unsupported object filtering with
the wildcard operator. Replace this expression with explicit references to the
matrix context fields that should be included in the artifact name, such as
`matrix.php` and `matrix.db`, combined using the `join()` function with explicit
array syntax like `join(array(matrix.php, matrix.db), ', ')` or by directly
interpolating the specific matrix variables separated by commas.
- Around line 27-42: The matrix strategy in the GitHub Actions workflow is
missing an extensions field that is referenced later at line 53 as ${{
matrix.extensions }}. Add an extensions field to the matrix definition alongside
the existing php, moodle-branch, and database fields. This field should include
the required PHP extensions for MariaDB connectivity (such as mysqli and
pdo_mysql) so they can be properly installed during the PHP setup step. Ensure
the extensions values are properly formatted as a list or array within the
matrix configuration.
---
Nitpick comments:
In @.github/workflows/moodle-ci.yaml:
- Around line 44-47: The checkout action at the "Check out repository code" step
does not explicitly set persist-credentials to false. Add persist-credentials:
false to the with section of the actions/checkout@v4 action alongside the
existing path: plugin configuration to prevent potential credential leakage if
the workflow is modified in the future.
- Around line 85-91: The "Moodle PHPDoc Checker" step has redundant failure
handling with both `|| exit 0` in the run command and `continue-on-error: true`
specified separately. Remove the `|| exit 0` from the moodle-plugin-ci phpdoc
command to use only `continue-on-error: true`, which is the clearer and simpler
approach for suppressing step failures.
- Line 45: Replace the semantic version tags for GitHub Actions with specific
commit SHAs to enhance supply chain security. At line 45, replace
`actions/checkout@v4` with the full commit SHA format (e.g.,
`actions/checkout@<full-commit-hash>`) and add a comment above indicating the
version (e.g., `# actions/checkout@v4.2.2`). Apply the same change pattern to
the other affected locations at lines 50 and 119, replacing their version tags
(`@v2` and others) with their corresponding commit SHAs and version comments. This
prevents potential tag tampering and ensures pinned actions are immutable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
There was a problem hiding this comment.
Nitpick comments (2)
.github/workflows/moodle-ci.yaml (2)
12-13: ⚡ Quick winConsider adding a job timeout.
The job has no
timeout-minutesspecified, which could allow hung jobs to consume runner minutes indefinitely. Adding a timeout (e.g., 60 minutes) would provide a safety net against resource exhaustion.Suggested configuration
test: runs-on: ubuntu-22.04 + timeout-minutes: 60Prompt 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/moodle-ci.yaml around lines 12 - 13, The test job in the workflow has no timeout protection, which could allow hung jobs to consume runner minutes indefinitely. Add a timeout-minutes property to the test job configuration to specify a maximum execution time (e.g., 60 minutes) that will automatically terminate the job if it exceeds the limit and prevent resource exhaustion.
60-67: 💤 Low valueConsider caching dependencies to improve workflow performance.
The workflow reinstalls Composer dependencies on every run. Caching the
~/.composer/cachedirectory and npm packages would reduce build times and external API load.Example caching configuration
Add a caching step before "Initialise moodle-plugin-ci":
- name: Cache Composer dependencies uses: actions/cache@v4 with: path: ~/.composer/cache key: ${{ runner.os }}-composer-${{ hashFiles('**/composer.lock') }} restore-keys: | ${{ runner.os }}-composer-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/moodle-ci.yaml around lines 60 - 67, Add a caching step before the "Initialise moodle-plugin-ci" step to reduce build times by avoiding reinstalling Composer dependencies on every workflow run. Use the actions/cache@v4 action to cache the ~/.composer/cache directory with a key based on the runner OS and a hash of composer.lock files, and include restore-keys to allow partial cache hits. This will preserve cached Composer dependencies across workflow runs while ensuring cache invalidation when dependencies change.
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.
Nitpick comments:
In @.github/workflows/moodle-ci.yaml:
- Around line 12-13: The test job in the workflow has no timeout protection,
which could allow hung jobs to consume runner minutes indefinitely. Add a
timeout-minutes property to the test job configuration to specify a maximum
execution time (e.g., 60 minutes) that will automatically terminate the job if
it exceeds the limit and prevent resource exhaustion.
- Around line 60-67: Add a caching step before the "Initialise moodle-plugin-ci"
step to reduce build times by avoiding reinstalling Composer dependencies on
every workflow run. Use the actions/cache@v4 action to cache the
~/.composer/cache directory with a key based on the runner OS and a hash of
composer.lock files, and include restore-keys to allow partial cache hits. This
will preserve cached Composer dependencies across workflow runs while ensuring
cache invalidation when dependencies change.
Adds GitHub Actions CI workflow for automated testing (PHP lint, code checker, PHPUnit, Behat). Only PHP 8.4 is used as this is a simple plugin.