diff --git a/.github/workflows/r-package-check.yml b/.github/workflows/r-package-check.yml new file mode 100644 index 0000000000..221d66a838 --- /dev/null +++ b/.github/workflows/r-package-check.yml @@ -0,0 +1,155 @@ +# Reusable R CMD check (workflow_call), derived from +# https://github.com/r-lib/actions/tree/v2/examples +# +# Consolidates the near-identical R-CMD-check.yaml files kaefa and nonnest2 +# each carried (r-lib's standard actions/checkout -> setup-pandoc -> +# [setup-tinytex] -> setup-r -> setup-r-dependencies -> check-r-package +# sequence). See docs/adr/0023-r-cmd-check-reusable-workflow-consolidation.md +# and docs/doctoring/r-cmd-check-reusable-workflow-consolidation.md for the +# per-repo field audit behind these inputs. +# +# The `on: push/pull_request` trigger stays in each calling repo's own thin +# workflow file -- a workflow_call target cannot also be the thing GitHub +# triggers directly on push/PR. +# +# Example caller (.github/workflows/R-CMD-check.yaml in a product repo). +# Pin `uses:` to this file's exact commit SHA, not @main: an unpinned mutable +# ref would run an unreviewed central change against every PR check in the +# calling repo (see dependency-review.yml's own header comment and +# docs/doctoring/dependency-review-reusable-workflow-consolidation.md for the +# incident that established this as the required pattern for every reusable +# workflow caller in this org). If the calling repo's branch protection +# requires a status check literally named after the old standalone job, +# converting to `uses:` here will rename the published check to +# " / R-CMD-check" and silently break that required check -- +# check for this before or immediately after merging a caller. +# +# name: R-CMD-check +# on: +# push: +# branches: [main, master] +# pull_request: +# branches: [main, master] +# jobs: +# R-CMD-check: +# uses: ContextualWisdomLab/.github/.github/workflows/r-package-check.yml@ +# with: +# needs_tinytex: true # only if the package builds a PDF vignette +# +name: Reusable R CMD check + +on: + workflow_call: + inputs: + r_matrix: + description: >- + JSON array of {os, r, http-user-agent?} objects for + strategy.matrix.config. Default is a single ubuntu-latest/release + leg; override with a JSON array for a multi-OS/multi-R-version + matrix. + required: false + type: string + default: '[{"os": "ubuntu-latest", "r": "release"}]' + needs_tinytex: + description: "Install r-lib/actions/setup-tinytex before setup-r (needed for a PDF vignette build)." + required: false + type: boolean + default: false + extra_packages: + description: "Value forwarded to setup-r-dependencies's extra-packages input." + required: false + type: string + default: "any::rcmdcheck" + check_args: + description: >- + Value forwarded to check-r-package's args input. Default matches + that action's own upstream default + (c("--no-manual", "--as-cran")); override to change what + rcmdcheck runs (e.g. to skip re-running tests already run by a + bounded pre-check test file). + required: false + type: string + default: 'c("--no-manual", "--as-cran")' + install_package_before_pre_check: + description: >- + Install the current package from source before the optional fixed + testthat pre-check. This is a boolean capability, not caller-authored + shell source. + required: false + type: boolean + default: false + pre_check_test_file: + description: >- + Optional repository-relative testthat file under tests/testthat/ + ending in .R. The value is passed as data through an environment + variable and is never evaluated as shell source. + required: false + type: string + default: "" + +permissions: + contents: read + +jobs: + R-CMD-check: + runs-on: ${{ matrix.config.os }} + name: ${{ matrix.config.os }} (${{ matrix.config.r }}) + + strategy: + fail-fast: false + matrix: + config: ${{ fromJSON(inputs.r_matrix) }} + + env: + GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }} + R_KEEP_PKG_SOURCE: yes + + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + + - uses: r-lib/actions/setup-pandoc@6f6e5bc62fba3a704f74e7ad7ef7676c5c6a2590 # v2 + + - if: inputs.needs_tinytex + uses: r-lib/actions/setup-tinytex@6f6e5bc62fba3a704f74e7ad7ef7676c5c6a2590 # v2 + + - uses: r-lib/actions/setup-r@6f6e5bc62fba3a704f74e7ad7ef7676c5c6a2590 # v2 + with: + r-version: ${{ matrix.config.r }} + http-user-agent: ${{ matrix.config['http-user-agent'] }} + use-public-rspm: true + + - uses: r-lib/actions/setup-r-dependencies@6f6e5bc62fba3a704f74e7ad7ef7676c5c6a2590 # v2 + with: + extra-packages: ${{ inputs.extra_packages }} + needs: check + + - if: inputs.pre_check_test_file != '' && inputs.install_package_before_pre_check + name: Install package for bounded pre-check + run: Rscript -e 'install.packages(".", repos = NULL, type = "source")' + shell: bash + + - if: inputs.pre_check_test_file != '' + name: Run bounded testthat pre-check + env: + PRE_CHECK_TEST_FILE: ${{ inputs.pre_check_test_file }} + run: | + case "$PRE_CHECK_TEST_FILE" in + tests/testthat/*.R) ;; + *) + echo "::error::pre_check_test_file must be a repository-relative tests/testthat/*.R path" + exit 1 + ;; + esac + if [[ "$PRE_CHECK_TEST_FILE" == *".."* || "$PRE_CHECK_TEST_FILE" == /* || "$PRE_CHECK_TEST_FILE" == *$'\n'* || "$PRE_CHECK_TEST_FILE" == *$'\r'* ]]; then + echo "::error::pre_check_test_file contains a forbidden path/control sequence" + exit 1 + fi + Rscript -e 'testthat::test_file(Sys.getenv("PRE_CHECK_TEST_FILE"))' + shell: bash + + - uses: r-lib/actions/check-r-package@6f6e5bc62fba3a704f74e7ad7ef7676c5c6a2590 # v2 + with: + args: ${{ inputs.check_args }} + build_args: 'c("--no-manual")' + error-on: '"error"' + upload-snapshots: true diff --git a/docs/adr/0023-r-cmd-check-reusable-workflow-consolidation.md b/docs/adr/0023-r-cmd-check-reusable-workflow-consolidation.md new file mode 100644 index 0000000000..05f8b92f79 --- /dev/null +++ b/docs/adr/0023-r-cmd-check-reusable-workflow-consolidation.md @@ -0,0 +1,49 @@ +# ADR-0023: Consolidate kaefa/nonnest2 R-CMD-check.yaml into one reusable workflow + +- **Status:** Proposed +- **Date:** 2026-09-02 +- **Scope:** `ContextualWisdomLab/.github` reusable R package CI; consumers `ContextualWisdomLab/kaefa` and `ContextualWisdomLab/nonnest2` + +## Problem + +`ContextualWisdomLab/kaefa` and `ContextualWisdomLab/nonnest2` carry near-identical R-CMD-check workflows derived from the r-lib Actions examples. The shared sequence is checkout → Pandoc → optional TinyTeX → R setup → dependency setup → optional repository-specific regression → `check-r-package`. Copying that sequence creates action-pin, permission, and behavior drift. + +A first reusable-workflow implementation exposed the repository-specific regression as a free-form `pre_check_script` string and interpolated it directly into `run:`. Current-head security review correctly identified that design as a privileged-code boundary defect: a reusable caller could supply arbitrary shell source to a job that receives the caller repository token. Consolidation does not justify transferring executable authority from a consumer into a centrally trusted workflow. + +## Decision + +1. `ContextualWisdomLab/.github/.github/workflows/r-package-check.yml` is the canonical reusable owner for the shared R-CMD-check sequence. +2. The reusable interface is data/capability oriented, not shell oriented. It accepts: + - `r_matrix`: JSON strategy matrix; + - `needs_tinytex`: boolean capability; + - `extra_packages`: dependency input forwarded to r-lib Actions; + - `check_args`: R CMD check arguments; + - `install_package_before_pre_check`: boolean capability for the known kaefa regression shape; + - `pre_check_test_file`: repository-relative `tests/testthat/*.R` path passed as data. +3. Free-form `pre_check_script` is forbidden. The workflow owns the only executable pre-check commands: an optional fixed `install.packages(".", ...)` invocation and a fixed `testthat::test_file(Sys.getenv("PRE_CHECK_TEST_FILE"))` invocation. +4. `pre_check_test_file` fails closed unless it is a relative `tests/testthat/*.R` path and contains no parent traversal, absolute-path prefix, carriage return, or newline. The path enters the shell only through an environment variable; it is never evaluated as shell source. +5. Uniform security/supply-chain fields remain centrally owned and non-parameterized: `permissions: contents: read`, `GITHUB_PAT`, `R_KEEP_PKG_SOURCE`, `build_args`, `error-on`, upload behavior, and immutable action SHAs. +6. Consumer trigger branches remain in each repository's thin caller. Consumers must pin `uses:` to the immutable protected-main commit containing the reusable workflow; mutable `@main`, PR heads, and branch URLs are not production dependency authority. +7. The current proposal remains **Proposed** until this exact candidate passes repository tests/security/review and integrates through protected `main`. Only then may consumer PRs pin the resulting protected-main SHA and reacquire their own exact-head evidence. + +## Alternatives considered + +- **Keep copied workflows.** Rejected because two already-identical control surfaces drift independently and duplicate maintenance/security review. +- **Free-form shell input.** Rejected because it turns caller data into executable commands in a centrally trusted job. +- **Parameterize action SHAs or permissions.** Rejected because supply-chain and token authority belong to the reusable workflow owner, not individual consumers. +- **Hard-code kaefa-specific file names centrally.** Rejected because the reusable owner should expose the minimum bounded semantic input needed by multiple products, not own product test identity. +- **Consume an unreleased PR-head version from product callers.** Rejected because consumers may use only protected/released immutable owner contracts. + +## Invariants and failure scenarios + +- A malicious or compromised caller cannot make the central job execute arbitrary Bash through an input. +- An invalid test-file path fails before R execution. +- A caller cannot elevate token permissions through the reusable workflow. +- If protected-main publication has not occurred, consumer adoption remains blocked rather than falling back to a mutable ref. +- Changing the caller to a reusable job may change the published check-context name; consumer branch/ruleset requirements must be re-read before adoption and repaired at the owning ruleset rather than silently weakening protection. + +## Consequences and follow-up + +The central workflow becomes a small reusable CI contract while product repositories retain only triggers and bounded product-specific values. `ContextualWisdomLab/kaefa#84` must replace its former shell input with `install_package_before_pre_check: true` and `pre_check_test_file: tests/testthat/test-zh-misfit-decision-rule.R`, then pin the eventual protected-main SHA. `ContextualWisdomLab/nonnest2#119` must likewise pin the protected-main SHA. Both consumer PRs remain non-authoritative until the owner integrates and their own current-head gates pass. + +The executable regression in `tests/test_r_package_check_reusable_workflow_contract.py` permanently forbids reintroducing caller-authored shell source and verifies the bounded pre-check path. diff --git a/docs/doctoring/r-cmd-check-reusable-workflow-consolidation.md b/docs/doctoring/r-cmd-check-reusable-workflow-consolidation.md new file mode 100644 index 0000000000..614c5b4ab7 --- /dev/null +++ b/docs/doctoring/r-cmd-check-reusable-workflow-consolidation.md @@ -0,0 +1,55 @@ +# R-CMD-check reusable workflow consolidation + +## Current authority + +This record describes the Proposed owner change in `ContextualWisdomLab/.github#1716`. Protected `main` remains production authority until the exact candidate integrates. Consumer PRs in `ContextualWisdomLab/kaefa` and `ContextualWisdomLab/nonnest2` must not consume this PR branch or mutable `@main`; after integration they pin the exact protected-main commit that contains the reusable workflow. + +## Original duplication + +`ContextualWisdomLab/kaefa` and `ContextualWisdomLab/nonnest2` both derived their R-CMD-check workflow from the r-lib Actions examples. Their common sequence and common authority fields justified a canonical reusable owner. Their real differences are bounded data/capabilities: trigger branches, R matrix, TinyTeX requirement, extra R packages, check arguments, and kaefa's one testthat regression. + +The action pins selected by the proposal are `actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1` and `r-lib/actions/*@6f6e5bc62fba3a704f74e7ad7ef7676c5c6a2590`. `permissions: contents: read`, `GITHUB_PAT`, `R_KEEP_PKG_SOURCE`, `build_args`, `error-on`, and snapshot-upload behavior remain owned centrally rather than becoming consumer inputs. + +## Security RCA: free-form pre-check shell + +The first candidate represented kaefa's two-command regression as a string input named `pre_check_script` and executed it with `run: ${{ inputs.pre_check_script }}`. Devin current-head review identified the resulting security boundary defect: reusable-workflow callers could provide arbitrary Bash source to a central job that receives the caller repository token. + +This is a canonical-owner defect, not a finding to suppress or merely document. The repair lineage on 2026-09-02 is: + +- RED commit `5e838ab35d062faa488b03ae78f9f8d84447e223`: adds an executable contract forbidding `pre_check_script`/caller-authored `run:` and requiring a bounded test-file data path; +- production commit `931c8f32a2e5e743ca0fbdee3d6728170ff2b273`: removes arbitrary shell input and introduces `install_package_before_pre_check` plus `pre_check_test_file`; +- contract-alignment commit `6ca3080326f3498904d6222c60089e35a050b848`: verifies step order, capability gates, environment-data binding, and fail-closed path checks on the repaired source. + +The repaired workflow owns its executable commands. When requested, it runs a fixed package installation command. The optional test file is passed only as `PRE_CHECK_TEST_FILE`, must match repository-relative `tests/testthat/*.R`, and is rejected for parent traversal, absolute-path prefixes, carriage returns, or newlines before the fixed `testthat::test_file(Sys.getenv("PRE_CHECK_TEST_FILE"))` command executes. No consumer string is evaluated as shell source. + +## Consumer equivalence + +The bounded replacement preserves kaefa's valid behavior without preserving the unsafe representation. Its former commands were: + +1. install the current package from source; +2. run `tests/testthat/test-zh-misfit-decision-rule.R` through testthat. + +The equivalent bounded caller values are: + +- `install_package_before_pre_check: true`; +- `pre_check_test_file: tests/testthat/test-zh-misfit-decision-rule.R`. + +Kaefa's five-leg R matrix, `any::rcmdcheck` + `any::testthat`, and `c("--no-manual", "--no-tests")` remain data inputs. Nonnest2 needs no pre-check capability and keeps its own trigger branches/TinyTeX behavior. Each consumer must pin the eventual owner protected-main SHA and regenerate its own current-head evidence. + +## Validation contract + +`tests/test_r_package_check_reusable_workflow_contract.py` checks the six bounded inputs, optional-step gates, immutable action pins, uniform central fields, matrix binding, absence of free-form shell input, and the fail-closed test-file grammar. Repository-wide pytest/coverage, docstring checks, actionlint, security workflows, and current-head independent review remain merge evidence only when they execute on the unchanged exact current head; predecessor results are historical evidence, not transferable approval. + +The unresolved Devin thread on the vulnerable implementation must remain unresolved until exact-head evidence proves the repaired successor. Queue saturation is not authority to bypass this substantive security finding. + +## Context and standards + +Reusable workflows establish an execution boundary: GitHub explicitly documents that called workflows receive permissions constrained by the caller and that permissions cannot be elevated through the call chain. This repair additionally minimizes the command surface so caller-controlled values remain data rather than command text. Shell/path validation here is defense in depth; the primary design rule is that the workflow itself owns executable source. + +## References (APA 7th edition) + +GitHub, Inc. (n.d.). *Reusing workflows*. GitHub Docs. Retrieved September 2, 2026, from https://docs.github.com/en/actions/how-tos/reuse-automations/reuse-workflows + +GitHub, Inc. (n.d.). *Workflow syntax for GitHub Actions*. GitHub Docs. Retrieved September 2, 2026, from https://docs.github.com/en/actions/reference/workflows-and-actions/workflow-syntax + +r-lib. (n.d.). *actions: GitHub Actions for the R community* [Computer software]. GitHub. Retrieved September 2, 2026, from https://github.com/r-lib/actions/tree/v2/examples diff --git a/tests/test_r_package_check_reusable_workflow_contract.py b/tests/test_r_package_check_reusable_workflow_contract.py new file mode 100644 index 0000000000..2d3ad49711 --- /dev/null +++ b/tests/test_r_package_check_reusable_workflow_contract.py @@ -0,0 +1,131 @@ +"""Contract for the reusable R-CMD-check workflow. + +Replaces kaefa's and nonnest2's near-identical, hand-copied +``R-CMD-check.yaml`` files with one reusable ``workflow_call`` workflow, +``.github/workflows/r-package-check.yml``, plus a thin caller left in each +product repository. See +``docs/doctoring/r-cmd-check-reusable-workflow-consolidation.md`` and +``docs/adr/0023-r-cmd-check-reusable-workflow-consolidation.md`` for why. +""" + +from __future__ import annotations + +from pathlib import Path + +_WORKFLOW = Path(".github/workflows/r-package-check.yml") + +_R_LIB_PIN = "6f6e5bc62fba3a704f74e7ad7ef7676c5c6a2590" +_CHECKOUT_PIN = "3d3c42e5aac5ba805825da76410c181273ba90b1" + + +def _workflow_text() -> str: + """Read the reusable R-CMD-check workflow as UTF-8 text.""" + return _WORKFLOW.read_text(encoding="utf-8") + + +def test_declares_workflow_call_with_six_inputs_and_recorded_defaults() -> None: + """Every genuinely varying caller field is data, never executable shell source.""" + workflow = _workflow_text() + assert "on:\n workflow_call:\n inputs:" in workflow + for name in ( + "r_matrix:", + "needs_tinytex:", + "extra_packages:", + "check_args:", + "install_package_before_pre_check:", + "pre_check_test_file:", + ): + assert name in workflow + + assert 'default: \'[{"os": "ubuntu-latest", "r": "release"}]\'' in workflow + assert workflow.count("default: false") >= 2 + assert 'default: "any::rcmdcheck"' in workflow + assert "default: 'c(\"--no-manual\", \"--as-cran\")'" in workflow + assert 'default: ""' in workflow + + +def test_step_order_matches_the_r_lib_template_sequence() -> None: + """checkout -> pandoc -> [tinytex] -> setup-r -> deps -> bounded pre-check -> check.""" + workflow = _workflow_text() + order = [ + "actions/checkout@", + "r-lib/actions/setup-pandoc@", + "r-lib/actions/setup-tinytex@", + "r-lib/actions/setup-r@", + "r-lib/actions/setup-r-dependencies@", + "Install package for bounded pre-check", + "Run bounded testthat pre-check", + "r-lib/actions/check-r-package@", + ] + positions = [workflow.index(marker) for marker in order] + assert positions == sorted(positions), "steps are out of order" + + +def test_optional_steps_are_gated_on_bounded_inputs() -> None: + """Optional setup and pre-check steps run only for explicit bounded capabilities.""" + workflow = _workflow_text() + assert ( + "- if: inputs.needs_tinytex\n uses: r-lib/actions/setup-tinytex@" + in workflow + ) + assert ( + "- if: inputs.pre_check_test_file != '' && inputs.install_package_before_pre_check\n" + " name: Install package for bounded pre-check" + in workflow + ) + assert ( + "- if: inputs.pre_check_test_file != ''\n" + " name: Run bounded testthat pre-check" + in workflow + ) + assert "PRE_CHECK_TEST_FILE: ${{ inputs.pre_check_test_file }}" in workflow + assert 'testthat::test_file(Sys.getenv("PRE_CHECK_TEST_FILE"))' in workflow + + +def test_action_pins_are_uniform_and_current() -> None: + """Every r-lib step and checkout share one current pin, not per-caller drift.""" + workflow = _workflow_text() + assert workflow.count(_R_LIB_PIN) == 5 # pandoc, tinytex, setup-r, deps, check + assert f"actions/checkout@{_CHECKOUT_PIN}" in workflow + assert f"r-lib/actions/setup-pandoc@{_R_LIB_PIN}" in workflow + assert f"r-lib/actions/setup-tinytex@{_R_LIB_PIN}" in workflow + assert f"r-lib/actions/setup-r@{_R_LIB_PIN}" in workflow + assert f"r-lib/actions/setup-r-dependencies@{_R_LIB_PIN}" in workflow + assert f"r-lib/actions/check-r-package@{_R_LIB_PIN}" in workflow + + +def test_uniform_fields_are_hardcoded_not_parameterized() -> None: + """Fields byte-identical across both originals stay static, not inputs.""" + workflow = _workflow_text() + assert "permissions:\n contents: read" in workflow + assert "GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }}" in workflow + assert "R_KEEP_PKG_SOURCE: yes" in workflow + assert "build_args: 'c(\"--no-manual\")'" in workflow + assert "error-on: '\"error\"'" in workflow + assert "upload-snapshots: true" in workflow + assert "args: ${{ inputs.check_args }}" in workflow + assert "extra-packages: ${{ inputs.extra_packages }}" in workflow + + +def test_matrix_is_driven_by_the_r_matrix_input() -> None: + """The strategy matrix must come from fromJSON(inputs.r_matrix), not a fixed list.""" + workflow = _workflow_text() + assert "config: ${{ fromJSON(inputs.r_matrix) }}" in workflow + assert "runs-on: ${{ matrix.config.os }}" in workflow + assert "r-version: ${{ matrix.config.r }}" in workflow + assert "http-user-agent: ${{ matrix.config['http-user-agent'] }}" in workflow + + +def test_pre_check_hook_is_bounded_data_not_caller_shell_source() -> None: + """A reusable caller must not inject arbitrary Bash source into the trusted job.""" + workflow = _workflow_text() + assert "pre_check_script:" not in workflow + assert "run: ${{ inputs.pre_check_script }}" not in workflow + assert "pre_check_test_file:" in workflow + assert "install_package_before_pre_check:" in workflow + assert "PRE_CHECK_TEST_FILE: ${{ inputs.pre_check_test_file }}" in workflow + assert 'case "$PRE_CHECK_TEST_FILE" in' in workflow + assert "tests/testthat/*.R" in workflow + assert '"$PRE_CHECK_TEST_FILE" == *".."*' in workflow + assert '"$PRE_CHECK_TEST_FILE" == /*' in workflow + assert 'testthat::test_file(Sys.getenv("PRE_CHECK_TEST_FILE"))' in workflow