feat(workflows): reusable R-CMD-check for kaefa/nonnest2 - #1716
Merged
Conversation
kaefa and nonnest2 each carried a hand-copied R-CMD-check.yaml generated from the same upstream r-lib template. Consolidate the shared checkout -> setup-pandoc -> [setup-tinytex] -> setup-r -> setup-r-dependencies -> check-r-package sequence into one workflow_call workflow with inputs for the fields that genuinely vary per repo (r_matrix, needs_tinytex, extra_packages, check_args, pre_check_script). See docs/adr/0023-r-cmd-check-reusable-workflow-consolidation.md and docs/doctoring/r-cmd-check-reusable-workflow-consolidation.md for the full field-by-field audit, including two non-uniform fields (extra-packages, check-r-package args) the initial survey missed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Warning Review limit reachedNext included review available in 33 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (4)
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 |
This was referenced Sep 2, 2026
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
seonghobae
added
area: ci-cd
ChatGPT Codex Connector
The dependency-review.yml consolidation's caller PRs surfaced a real Devin security finding: uses: <reusable-workflow>@main runs an unreviewed central change against every caller's PR checks with no review in the calling repo. Fixed there (all four callers pinned to a commit SHA); apply the same correction to this not-yet-merged reusable workflow's own documented example before any caller PR copies the unsafe pattern. Also notes the separate required-status-check-name gotcha (converting a job to uses: renames its published check) to check for in each caller repo before merging. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Owner goal
Consolidate the duplicated R-CMD-check implementation in
ContextualWisdomLab/kaefaandContextualWisdomLab/nonnest2behind one canonical reusable workflow without transferring executable authority from consumers into the trusted central job.Security RCA and test-first repair
Devin review found a substantive defect in the first design: caller input
pre_check_scriptwas interpolated directly into a Bashrun:block, so a compromised caller could execute arbitrary commands with the reusable job's repository token.The canonical owner repair is test-first:
5e838ab35d062faa488b03ae78f9f8d84447e223: executable regression forbidspre_check_script/ caller-authored shell and requires bounded data inputs.931c8f32a2e5e743ca0fbdee3d6728170ff2b273: replaces arbitrary shell input withinstall_package_before_pre_checkandpre_check_test_file; the workflow owns fixed R commands and validates the path before execution.6ca3080326f3498904d6222c60089e35a050b848: updates step/input/path-validation assertions around the repaired source.0747ae12b0ec77f2275e113f9af8630dc8a41bf0/8691ac6c7365c1ee99148bbb5bfbd5d609be0c3c: records the trust-boundary decision and keeps ADR-0023 Proposed until protected integration.The bounded interface now consists of
r_matrix,needs_tinytex,extra_packages,check_args,install_package_before_pre_check, andpre_check_test_file.pre_check_test_filemust be a repository-relativetests/testthat/*.Rpath; parent traversal, absolute paths, CR, and LF fail closed. Consumer data is passed through an environment variable to a fixedtestthat::test_file(...)command and is never evaluated as shell source.Current-main reconciliation
Protected
mainadvanced concurrently. The writer was advanced without force-push/destructive rebase to exact head9ec686e5f6753904ae5eecc1f35aceffb44280c3, a two-parent merge preserving protectedmain@63bf49835da44aa8257eb76a92368e6485ae6e94and the repaired writer history. Fresh current-main comparison is ahead-only (behind_by=0) and the effective delta is exactly four files: the reusable workflow, ADR-0023, doctoring, and its executable contract test.Review and consumer stack
The original arbitrary-shell security finding is resolved on the repaired exact head. Current Devin Review and CodeRabbit commit statuses are successful, and all review threads have been resolved only after the relevant source/contract or dependency-order evidence was present.
ContextualWisdomLab/kaefa#84remains a live dependent and must replace its retired shell input withinstall_package_before_pre_check: truepluspre_check_test_file: tests/testthat/test-zh-misfit-decision-rule.R.ContextualWisdomLab/nonnest2#119remains the second live dependent.@mainor this PR head as production authority. After this owner PR integrates, both caller branches must pin the exact protected-main commit carryingr-package-check.ymland regenerate their own exact-head evidence before either migration merges.Merge boundary
Ordinary auto-merge is armed, not administrator bypass. Current exact-head security/quality workflow runs are queued/pending in the saturated Actions fleet; predecessor results do not transfer and queued evidence is not GREEN. Merge only when ordinary protected admission is satisfied on the unchanged exact head. The consumer PRs remain open until post-publication immutable-pin repair and their own current-head admission complete.