fix(ci): harden the AUR publish pipeline and repair its distribution CI - #297
Conversation
Le dépôt est public et la génération de la clé de déploiement AUR laisse aur_ci, aur_ci.pub et aur_known_hosts à la racine. La privée est détruite après enregistrement dans les secrets, mais rien n'empêchait un git add -A de la committer entre-temps.
Un tag git est mutable et cette action tierce reçoit WINGET_ACC_TOKEN : un tag repointé exfiltrerait le token sans qu'aucun diff n'apparaisse ici. v2 resolves to 4ffc7888bffd451b357355dc214d43bb9f23917e.
Le PKGBUILD publié sur l'AUR n'est pas versionné ici : il appartient au mainteneur du paquet et peut changer sans passer par une PR. Le workflow le clonait, le patchait et le repoussait sans jamais regarder ce qu'il contenait. - makepkg *source* le PKGBUILD ; il s'exécutait donc dans un runner qui détenait déjà la clé de déploiement. Clone en HTTPS, clé écrite seulement après validation. - Audit du préambule, allowlist des sources sur notre dépôt, et refus de pousser un diff qui déborde de pkgver/pkgrel/sha256sums. - Le tag de workflow_dispatch est du texte libre et finissait dans une expression sed : v1.0|e id| donnait une exécution de shell. Regex stricte, qui bloque aussi la publication d'une RC (pkgver interdit le tiret). - Un seul checksum était réécrit ; celui du LICENSE, épinglé sur le tag, périmait en silence et cassait makepkg chez les utilisateurs. - L'étape d'installation ne pouvait pas aboutir : pacman-package-manager ne fournit pas makepkg mais s'installe sans erreur, rendant le repli mort. makepkg seul ne suffit pas non plus, il résout pacman par type -P. Jamais détecté car le job n'avait jamais dépassé son if: — vars.AUR_PACKAGE_NAME n'existait pas, les 10 dernières releases sont skipped.
|
Warning Review limit reached
Next review available in: 1 minute You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe AUR workflow now validates tags, PKGBUILD declarations, source URLs, checksums, generated metadata, and publication diffs before pushing. SSH setup occurs after validation. The WinGet workflow pins its action to a commit SHA, and ChangesAUR publication validation
WinGet action pinning
Estimated code review effort: 4 (Complex) | ~45 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant GitHubActions
participant GitHubReleaseAssets
participant Makepkg
participant AURSSHRemote
GitHubActions->>GitHubReleaseAssets: download and hash all declared sources
GitHubActions->>Makepkg: build package and regenerate .SRCINFO
GitHubActions->>AURSSHRemote: push validated package changes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.github/workflows/aur-publish.yml (1)
118-131: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAdd process substitution to the rejected preamble constructs.
The pattern list catches
$(, backticks, and a set of command names. It does not catch<(or>(. Bash evaluates process substitution in the preamble the same way as command substitution, sosource=(<(curl ...))or similar top-level use would pass this audit.The audit is defense in depth, and
makepkgstill runs after it, so this is not a blocker. Adding the two tokens keeps the stated guarantee intact.🛡️ Proposed addition to the pattern list
done <<'PATTERNS' \$\( ` + [<>]\( (^|[;&|[:space:]])(eval|source|curl|wget|bash|sh|python3?)[[:space:]] PATTERNS🤖 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/aur-publish.yml around lines 118 - 131, Update the PATTERNS list in the PKGBUILD preamble audit to reject both Bash process-substitution tokens, <( and >(. Keep the existing command-substitution, backtick, and command-name checks 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/aur-publish.yml:
- Around line 202-214: Harden the URL validation in the source-fetch loop around
SUMS and URLS: reject URLs containing dot-segment path components before
fetching, and prevent redirects from bypassing the allowlist by removing curl’s
redirect-following behavior or validating the final effective URL against
ALLOWED_SOURCE_PREFIX. Ensure only the validated repository origin is used to
calculate checksums.
- Around line 193-200: Before writing the regenerated sha256sums array in the
workflow’s URL/SUMS processing, compare the source-entry count with the SUMS
count and fail with an error if they differ. Ensure local or non-URL source
entries are represented so the generated sha256sums array preserves the source
array length and prevents publishing an invalid PKGBUILD.
---
Nitpick comments:
In @.github/workflows/aur-publish.yml:
- Around line 118-131: Update the PATTERNS list in the PKGBUILD preamble audit
to reject both Bash process-substitution tokens, <( and >(. Keep the
existing command-substitution, backtick, and command-name checks unchanged.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: bccc6a5f-40ca-48f4-8cd1-55e487b899a0
📒 Files selected for processing (3)
.github/workflows/aur-publish.yml.github/workflows/publish-winget.yml.gitignore
Permet d'exercer tout le pipeline sur un vrai runner — install de makepkg, clone, audit, bump, .SRCINFO, contrôle de diff — en s'arrêtant avant le push. La clé de déploiement n'est alors jamais écrite sur le disque. Nécessaire pour valider ce workflow avant sa première publication réelle : il n'a jamais dépassé son if: en dix releases.
- sha256sums pouvait devenir plus court que source=(). Une entrée locale et légitime (.install, .desktop) ne produit aucune URL ; on écrivait alors un tableau désaligné que makepkg génère sans broncher et que le contrôle de diff laisse passer, pour finir en échec de vérification chez chaque utilisateur. On compte les entrées et on refuse un formatage non reconnu. - Le test de préfixe de l'allowlist ne contraignait pas l'origine réellement contactée : curl normalise les segments .. avant la requête, donc .../getopenscreen/openscreen/../../attacker/repo/x passait et allait chercher le dépôt d'un tiers. Reproduit — l'URL effective devient bien github.com/attacker/repo. Segments .., pourcent, arobase et antislash refusés, et l'hôte d'arrivée est vérifié après redirection. - La liste de motifs du préambule ignorait la substitution de processus. 23 assertions passent, dont 15 négatives.
1 participant
Contexte
Mise en place de la publication AUR automatisée. Les secrets et variables sont déjà configurés côté GitHub (
AUR_SSH_PRIVATE_KEYen secret,AUR_KNOWN_HOSTSetAUR_PACKAGE_NAMEen variables) ; cette PR ne contient que le code.Point de départ : le job n'avait jamais tourné. Il était gardé par
vars.AUR_PACKAGE_NAME, qui n'existait pas — les 10 dernières releases sontskipped. Tout ce qui suit est donc du code jamais exécuté, dont deux bugs qui l'empêchaient purement et simplement d'aboutir.Le fait structurant : le PKGBUILD n'est pas versionné ici. Il vit sur l'AUR, appartient au mainteneur du paquet (
psychosomat), et peut changer sans passer par une PR. Le workflow le clonait, le patchait et le repoussait sans jamais regarder ce qu'il contenait.Corrections
Sécurité
makepkgsource le PKGBUILD tiers — il s'exécutait dans un runner détenant déjà la clé de déploiementgithub.com/getopenscreen/openscreen/+ refus d'un diff débordant depkgver/pkgrel/sha256sumsworkflow_dispatchest du texte libre et finissait dans une expressionsed:v1.0|e id|sortait de l'expression et exécutait du shell^v[0-9]+\.[0-9]+\.[0-9]+$pkgverinvalide (pkgverinterdit le tiret)winget-releaserépinglé sur un tag mutable alors qu'il reçoitWINGET_ACC_TOKENCorrectness
sedne touchait quesha256sums[0]. Celui du LICENSE est épinglé surraw/v${pkgver}/LICENSE: il périmait dès que ce fichier changeait, etmakepkgéchouait alors chez tous les utilisateurs, pas chez nous. Tous les checksums sont désormais recalculés depuis les URLs réelles.pacman-package-managerne fournit pasmakepkg(seulementpacman,pacman-conf,pacman-db-upgrade,pacman-key,repo-add) mais s'installe sans erreur, ce qui rendait le|| apt-get install makepkginatteignable — l'étape mourait sur « makepkg still missing after install ». Etmakepkgseul ne suffit pas non plus : il ne dépend pas du binairepacman, qu'il résout partype -Pau démarrage, et sort sinon sur « An unknown error has occurred ». Les deux paquets sont maintenant installés..pacmande la release.Vérification
Le code réel des blocs
run:est extrait du YAML et exécuté contre le vrai dépôt AUR — aucune logique n'est recopiée dans le harnais, il ne peut donc pas diverger.$(), backticks,pkgnamedétourné,sha256sumsmulti-lignes, source détournée vers un autre hôte, backdoor injectée danspackage(), et 5 tags malveillants.makepkg --printsrcinforéellement exécuté (6.0.2, l'image deubuntu-latest) :.SRCINFOcorrect pour1.9.0-1, 2 sources / 2 checksums.run:passentbash -naprès dédentation YAML.Deux bugs ont été trouvés dans mon propre code par ces tests et corrigés :
\bsource\brejetait la déclarationsource=()légitime, etmakepkg --version | head -1prenait un SIGPIPE quepipefailremontait en échec d'étape.Ce que ça ne couvre pas
workflow_dispatchsurv1.9.0(l'AUR est à1.7.0-2).package()modifié, mais il bloque dans ce cas au lieu de publier : un changement légitime en amont demandera une intervention manuelle. C'est le compromis assumé.🤖 Generated with Claude Code
Summary by CodeRabbit
Security & Reliability
Maintenance