ci: Use our own pr_title_lint.py instead of NPM commitlint [citest_skip]#338
ci: Use our own pr_title_lint.py instead of NPM commitlint [citest_skip]#338richm wants to merge 1 commit into
Conversation
There are a few problems with the NPM commitlint * NPM is problematic * Having to install NPM and commitlint and dependencies is slow and heavyweight * commitlint is overkill for our simple use case We have our own pr_title_lint.py which is small and works exactly for our use case. Enhance coderabbit checks for PRs and test cleanup Signed-off-by: Rich Megginson <rmeggins@redhat.com>
|
Warning Ignoring CodeRabbit configuration file changes. For security, only the configuration from the base branch is applied for open source repositories. 📝 WalkthroughWalkthroughThis PR updates ChangesCI Validation Configuration Updates
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.coderabbit.yaml (1)
42-49: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueFragile line-anchored reference for valid types.
Line 43 references a specific line number (
#L23) in an external file to document valid Conventional Commit types. Ifpr_title_lint.pyis updated upstream (lines added/removed), this anchor will silently point to the wrong line, misleading contributors/reviewers reading the requirement.Consider linking to the file itself (or a specific commit/tag) without the line anchor, or referencing the constant/variable name instead of a line number.
🤖 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 @.coderabbit.yaml around lines 42 - 49, The Conventional Commit types reference in the comment is tied to a fragile external line anchor, so update the guidance in .coderabbit.yaml to point to the source file, tag, or stable symbol instead of a specific line number. Keep the examples and valid-types note, but remove the `#L23` dependency so the reference remains correct as pr_title_lint.py changes upstream.
🤖 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/pr-title-lint.yml:
- Around line 25-26: The pr_title_lint.py download step is pulling from a
mutable upstream branch and does not fail cleanly on HTTP errors. Update the
curl command in the workflow’s install step to fetch from an immutable ref (a
pinned commit or tag) instead of main, and add curl fail behavior such as --fail
or --fail-with-body so CI stops immediately on 4xx/5xx responses before invoking
the script.
---
Nitpick comments:
In @.coderabbit.yaml:
- Around line 42-49: The Conventional Commit types reference in the comment is
tied to a fragile external line anchor, so update the guidance in
.coderabbit.yaml to point to the source file, tag, or stable symbol instead of a
specific line number. Keep the examples and valid-types note, but remove the
`#L23` dependency so the reference remains correct as pr_title_lint.py changes
upstream.
🪄 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 Plus
Run ID: 21044011-baff-492c-a358-2f7081344fb2
📒 Files selected for processing (3)
.coderabbit.yaml.commitlintrc.js.github/workflows/pr-title-lint.yml
💤 Files with no reviewable changes (1)
- .commitlintrc.js
| - name: Install pr_title_lint.py | ||
| run: curl -o pr_title_lint.py https://raw.githubusercontent.com/linux-system-roles/auto-maintenance/main/pr_title_lint.py |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
linux-system-roles auto-maintenance pr_title_lint.py latest commit checksum verification recommended practice for downloading scripts in CI
💡 Result:
The linux-system-roles/auto-maintenance repository is a collection of automation scripts and tools used to manage various linux-system-roles repositories [1]. Regarding the specific query about pr_title_lint.py, while auto-maintenance contains many utility scripts [1], standard PR title linting in the linux-system-roles ecosystem is typically handled via GitHub Actions—specifically using commitlint as shown in the linux-system-roles/template repository [2]. Recommended practices for downloading and executing scripts in CI pipelines emphasize security to prevent supply chain attacks, as piping directly from curl to shell (e.g., curl | bash) is considered a security risk [3][4]. The following practices are recommended: 1. Use Trusted Sources and Version Pinning: Whenever possible, use official plugins, package managers (apt, apk, npm), or container images [3][4]. If you must download a script, ensure it is from a trusted, immutable source and pin it to a specific version or commit [3]. 2. Verify Integrity via Checksums: Never execute a remote script without verification [3]. The standard practice is to: - Download the script to a temporary file [3]. - Compute its hash (e.g., sha256sum) [3][5]. - Compare the computed hash against a known, trusted checksum [3][5]. - Only execute the script if the verification succeeds [3][6][5]. Example CI pattern for checksum verification: curl -fsSL https://example.com/script.sh -o script.sh echo "EXPECTED_HASH script.sh" | sha256sum -c - if [ $? -eq 0 ]; then bash script.sh else echo "Checksum verification failed" exit 1 fi 3. Avoid Dynamic Scripts: If a script is generated dynamically, it is difficult to verify via a static checksum [5]. In such cases, prefer cloning the repository and running the script from a specific commit hash, which guarantees integrity through git's cryptographic signing rather than relying on a downloadable artifact checksum [7][5].
Citations:
- 1: https://github.com/linux-system-roles/auto-maintenance
- 2: https://github.com/linux-system-roles/template/blob/main/.github/workflows/pr-title-lint.yml
- 3: https://boostsecurityio.github.io/poutine/rules/unverified_script_exec/
- 4: https://zenn.dev/dk_/articles/7a87637abcfcda?locale=en
- 5: https://github.com/HomericIntelligence/AchaeanFleet/blob/d6786809336e47c2ac08d13033501955882f9c77/.github/workflows/verify-checksums.yml
- 6: https://checksum.sh/
- 7: https://threedots.tech/post/keeping-common-scripts-in-gitlab-ci/
🏁 Script executed:
#!/bin/bash
set -euo pipefail
git ls-files .github/workflows/pr-title-lint.yml .github/workflows
wc -l .github/workflows/pr-title-lint.yml
cat -n .github/workflows/pr-title-lint.yml | sed -n '1,120p'
printf '\n--- related references ---\n'
rg -n "pr_title_lint\.py|curl .*raw.githubusercontent.com|--fail|-f" .github/workflows . -g '!**/node_modules/**' -g '!**/dist/**' -g '!**/build/**'Repository: linux-system-roles/certificate
Length of output: 8607
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "curl .*raw\.githubusercontent\.com|curl .* -f|curl .*--fail|curl .* -L" .github/workflows -g '*.yml' -g '*.yaml'Repository: linux-system-roles/certificate
Length of output: 736
🌐 Web query:
curl --fail exit code HTTP 404 body saved without --fail documentation
💡 Result:
When using the --fail (or -f) option in curl, the program will exit with code 22 if an HTTP server returns an error response code of 400 or greater [1][2]. Crucially, the --fail option explicitly prevents curl from outputting or saving the response body for these errors [1][3]. In these cases, curl stops immediately after receiving the response headers and does not store the content [1][4]. If you need to both trigger an exit code on HTTP errors (400+) and save the response body (such as the HTML error page or error details), you should use the --fail-with-body option instead [1][5][4]. This option, introduced in curl version 7.76.0, functions as an alternative to --fail by allowing the body to be saved while still returning exit code 22 upon encountering a server error [1][4]. These two options, --fail and --fail-with-body, are mutually exclusive [1][6].
Citations:
- 1: https://curl.se/docs/manpage.html
- 2: https://manpages.debian.org/curl(1)
- 3: https://man7.org/linux/man-pages/man1/curl.1.html
- 4: https://daniel.haxx.se/blog/2021/02/11/curl-fail-with-body/
- 5: https://man.archlinux.org/man/curl.1.en.txt
- 6: https://manpages.opensuse.org/Leap-16.0/curl/curl.1.en.html
Pin the downloaded script to an immutable ref and fail fast on HTTP errors. Fetching from main means CI executes whatever upstream publishes next; pin a commit/tag, and use curl --fail (or --fail-with-body) so 4xx/5xx responses don’t turn into a confusing Python traceback.
🤖 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-title-lint.yml around lines 25 - 26, The
pr_title_lint.py download step is pulling from a mutable upstream branch and
does not fail cleanly on HTTP errors. Update the curl command in the workflow’s
install step to fetch from an immutable ref (a pinned commit or tag) instead of
main, and add curl fail behavior such as --fail or --fail-with-body so CI stops
immediately on 4xx/5xx responses before invoking the script.
1 participant
There are a few problems with the NPM commitlint
We have our own pr_title_lint.py which is small and works exactly for our use case.
Enhance coderabbit checks for PRs and test cleanup
Signed-off-by: Rich Megginson rmeggins@redhat.com
Summary by CodeRabbit