diff --git a/.github/workflows/dependency-review.yml b/.github/workflows/dependency-review.yml new file mode 100644 index 0000000000..d199dd36a0 --- /dev/null +++ b/.github/workflows/dependency-review.yml @@ -0,0 +1,139 @@ +# Reusable Dependency Review (workflow_call), consolidating the four +# near-identical dependency-review.yml files argos, mightyETL, newsdom-api, +# and scopeweave each carried independently. See +# docs/adr/0024-dependency-review-reusable-workflow-consolidation.md and +# docs/doctoring/dependency-review-reusable-workflow-consolidation.md for the +# per-repo field audit behind these inputs. +# +# The `on: pull_request` trigger (and any branch restriction) stays in each +# calling repo's own thin workflow file -- a workflow_call target cannot also +# be the thing GitHub triggers directly on pull_request. +# +# Dependency Review requires GitHub Dependency Graph (and, on private repos +# without GitHub Advanced Security, it is unavailable regardless of a repo's +# own settings). scopeweave's original workflow already detected this +# dynamically via the dependency-graph compare API instead of assuming from +# public/private repository status (mightyETL's original approach, which is +# wrong for a private repo that does have GHAS). This reusable workflow +# adopts the dynamic detection as the common, more-correct behavior for +# every caller, so no per-repo public/private input is needed. +# +# Example caller (.github/workflows/dependency-review.yml in a product repo): +# +# name: Dependency Review +# on: +# pull_request: +# concurrency: +# group: dependency-review-${{ github.event.pull_request.number || github.ref }} +# cancel-in-progress: true +# jobs: +# dependency-review: +# uses: ContextualWisdomLab/.github/.github/workflows/dependency-review.yml@main +# with: +# fail_on_severity: high +# allow_ghsas: "GHSA-69w3-r845-3855" + +name: Reusable Dependency Review + +on: + workflow_call: + inputs: + fail_on_severity: + description: "Value forwarded to dependency-review-action's fail-on-severity input." + required: false + type: string + default: "moderate" + allow_ghsas: + description: >- + Comma-or-newline-separated GHSA IDs forwarded to + dependency-review-action's allow-ghsas input. Empty (the default) + allows none. + required: false + type: string + default: "" + continue_on_error: + description: >- + Whether the dependency-review step itself is allowed to fail + without failing the job (argos's original behavior, which relies + on a separate blocking OSV-Scanner gate instead of this one). + Default false makes the dependency-review step itself blocking. + required: false + type: boolean + default: false + +permissions: + contents: read + pull-requests: read + +jobs: + dependency-review: + runs-on: ubuntu-latest + env: + # Opts every JS action this job runs (checkout, dependency-review-action) + # into the Node 24 actions runtime ahead of GitHub's default cutover, + # matching newsdom-api's original workflow -- applied uniformly here + # since it is a forward-compatibility setting, not a per-repo policy. + FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: true + steps: + - name: Checkout + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + persist-credentials: false + + - name: Check dependency graph availability + id: dependency_graph + env: + GH_TOKEN: ${{ github.token }} + BASE_SHA: ${{ github.event.pull_request.base.sha }} + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + REPOSITORY: ${{ github.repository }} + shell: bash + run: | + set -euo pipefail + + if [ "${{ github.event_name }}" != "pull_request" ]; then + echo "available=false" >>"$GITHUB_OUTPUT" + echo "Dependency review only runs as a hard gate for pull_request events." + exit 0 + fi + + api_url="${GITHUB_API_URL:-https://api.github.com}" + response_file="$(mktemp)" + status="$( + curl -fsS -o "$response_file" -w '%{http_code}' \ + -H "Accept: application/vnd.github+json" \ + -H "Authorization: Bearer ${GH_TOKEN}" \ + -H "X-GitHub-Api-Version: 2022-11-28" \ + "${api_url}/repos/${REPOSITORY}/dependency-graph/compare/${BASE_SHA}...${HEAD_SHA}" \ + || true + )" + + if [ "$status" = "200" ]; then + echo "available=true" >>"$GITHUB_OUTPUT" + exit 0 + fi + + if [ "$status" = "403" ] || [ "$status" = "404" ]; then + echo "::warning::Dependency graph compare returned HTTP ${status} for ${REPOSITORY}; skipping the dependency-review hard gate (GitHub Dependency Graph, or GitHub Advanced Security on a private repository, is unavailable)." + echo "available=false" >>"$GITHUB_OUTPUT" + exit 0 + fi + + echo "::error::Dependency graph availability check failed with HTTP ${status}. This is not a 'graph unavailable' response (403/404) -- treating it as a genuine failure instead of silently skipping the security gate." + cat "$response_file" + exit 1 + + - name: Dependency review + if: steps.dependency_graph.outputs.available == 'true' + continue-on-error: ${{ inputs.continue_on_error }} + uses: actions/dependency-review-action@a1d282b36b6f3519aa1f3fc636f609c47dddb294 # v5.0.0 + with: + fail-on-severity: ${{ inputs.fail_on_severity }} + allow-ghsas: ${{ inputs.allow_ghsas }} + comment-summary-in-pr: on-failure + + - name: Dependency graph unavailable note + if: steps.dependency_graph.outputs.available != 'true' && github.event_name == 'pull_request' + run: | + echo "Dependency Review requires GitHub Dependency Graph to be enabled for this repository (and, on private repositories, GitHub Advanced Security)." + echo "Other required dependency-vulnerability gates (OSV-Scanner, Scorecard) remain the blocking coverage until Dependency Graph is available here." diff --git a/docs/adr/0024-dependency-review-reusable-workflow-consolidation.md b/docs/adr/0024-dependency-review-reusable-workflow-consolidation.md new file mode 100644 index 0000000000..51bc203c37 --- /dev/null +++ b/docs/adr/0024-dependency-review-reusable-workflow-consolidation.md @@ -0,0 +1,120 @@ +# ADR-0024: Consolidate per-repo Dependency Review workflows into one reusable workflow + +- **Status:** Accepted +- **Date:** 2026-09-02 +- **Scope:** `.github/workflows/dependency-review.yml` (new, central, `workflow_call`); + thin callers in `argos`, `mightyETL`, `newsdom-api`, `scopeweave` + +## Context + +Four repositories each carried an independently hand-written +`dependency-review.yml` running `actions/dependency-review-action` on pull +requests: `argos`, `mightyETL`, `newsdom-api`, `scopeweave`. This is exactly +the drift `docs/CWL-MASTER-CONTEXT.md` §7 and this repo's own +"individual-repository workflow duplication" standardization effort target — +per-repo copies of the same control drift independently and cost bootup time +on every PR run. + +A field-by-field audit of all four files (2026-09-02) found: + +| Field | argos | mightyETL | newsdom-api | scopeweave | +| --- | --- | --- | --- | --- | +| `fail-on-severity` | `moderate` | `high` | unset (action default `low`) | `moderate` | +| `allow-ghsas` | none | none | `GHSA-69w3-r845-3855` | none | +| `comment-summary-in-pr` | unset | unset | unset | `on-failure` | +| step-level `continue-on-error` | `true` | unset (blocking) | unset (blocking) | unset (blocking) | +| Dependency Graph availability handling | none (always runs, no fallback) | static `github.event.repository.private` branch to a separate no-op job | none | dynamic API preflight (`dependency-graph/compare` HTTP status): 200 → run the gate, 403/404 → warn and skip, any other status → hard-fail the job | +| trigger scope | `pull_request: branches: [main, developmental]` | `pull_request` (all branches) | `pull_request` (all branches) | `pull_request` + `workflow_dispatch` | +| concurrency group | none | `${{ github.workflow }}-${{ github.event.pull_request.number \|\| github.ref }}` | none | `dependency-review-${{ github.event.pull_request.number \|\| github.ref }}` | +| `actions/checkout` pin | unpinned `@v4` | n/a (action doesn't need checkout) | SHA `3d3c42e5...` | SHA `9c091bb2...` (v7.0.0) | +| `dependency-review-action` pin | unpinned `@v4` | SHA `a1d282b3...` (v5.0.0) | SHA `a1d282b3...` | SHA `a1d282b3...` | +| `FORCE_JAVASCRIPT_ACTIONS_TO_NODE24` | unset | unset | `true` | unset | + +Two findings changed the design from a naive copy-paste consolidation: + +1. **Severity and the GHSA allowlist genuinely vary per repo** — these are + real policy differences (newsdom-api carries a documented upstream false + positive it allowlists; mightyETL runs a stricter `high`-only gate), not + accidental drift. They must stay per-caller inputs, not get silently + flattened to one value. +2. **mightyETL's public/private branch is the wrong generalization.** + `github.event.repository.private == false` assumes GHAS availability + tracks repository visibility, but a private repository can have GitHub + Advanced Security enabled (making Dependency Graph available) while a + public repository can still lack Dependency Graph in edge cases. scopeweave's + dynamic preflight — call the dependency-graph compare API directly and + check the HTTP status — checks the actual capability rather than inferring + it, and already existed independently in one of the four originals. This + ADR generalizes scopeweave's approach to all four callers rather than + mightyETL's, and drops the separate no-op fallback job in favor of one job + with a conditional step (the same job either runs the gate or emits the + unavailability note, never both, with no risk of the fallback job being + forgotten when Dependency Graph later becomes available). scopeweave's + preflight also distinguishes a confirmed-unavailable response (403/404 — + warn and skip) from any other unexpected HTTP status (500, an auth + failure, a transient GitHub API problem — hard-fail the job instead of + silently skipping the security gate); the reusable workflow preserves + that exact distinction rather than the simpler "any non-200 means + unavailable" behavior an initial draft of this workflow used, since + collapsing a real failure into "unavailable" would silently drop + coverage instead of surfacing the problem. +3. **`comment-summary-in-pr: on-failure` is a uniformly-beneficial UX + improvement, not a policy choice.** Only scopeweave's original set it + (posts the dependency-review findings as a PR comment when the gate + fails). It changes nothing about pass/fail semantics, only where a + failure's detail is surfaced, so it is hardcoded uniformly rather than + made an input — the other three repositories gain it for free. +4. **`FORCE_JAVASCRIPT_ACTIONS_TO_NODE24` is a forward-compatibility setting, + not a policy choice.** newsdom-api was the only original to set it, + opting its job into GitHub's Node 24 actions runtime ahead of the default + cutover for the JS actions it runs (`actions/checkout`, + `actions/dependency-review-action` — both JS actions in every one of the + four originals). There is no reason the other three repositories should + not also get this ahead of Node 20's eventual end-of-life, so it is + hardcoded uniformly in the reusable workflow's job `env`, not made an + input. + +## Decision + +Add `.github/workflows/dependency-review.yml` to `ContextualWisdomLab/.github` +as a `workflow_call` reusable workflow with three inputs for the +genuinely-varying fields: `fail_on_severity` (string, default `"moderate"`), +`allow_ghsas` (string, default `""`), and `continue_on_error` (boolean, +default `false`, for argos's non-blocking original behavior). The dynamic +Dependency Graph availability check (scopeweave's design) is hardcoded and +uniform for every caller — it is a correctness fix, not a policy choice, so +it does not need to be an input. + +Each of the four repositories keeps a thin caller workflow with its own +`on: pull_request` trigger (including argos's `branches:` restriction, which +cannot live inside a `workflow_call` target), a `concurrency` group (added to +argos and newsdom-api, which lacked one, bringing all four to the same +cancel-in-progress-on-repush posture used elsewhere in the org per the +concurrency-standardization pass this workflow-consolidation effort is part +of), and `with:` values reproducing that repository's original severity and +allowlist exactly. The old hand-written workflow bodies are deleted from each +repository in the same change, per this org's "repository-local copies are +drift sources, not repo-specific contracts" principle +(`README.md` policy summary; this repo's own `CLAUDE.md`). + +## Consequences + +- One place to fix a bug in the dependency-review logic (e.g. the + availability-detection curl call) instead of four. +- Each repository keeps its own severity/allowlist policy explicitly and + visibly in its own thin caller, not hidden in a shared default that could + silently loosen or tighten a repo's actual gate. +- argos and newsdom-api gain the cancel-in-progress concurrency group they + previously lacked, at no cost — a stale run for a superseded push no longer + keeps running or occupying a runner slot. +- `mightyETL`'s previous two-job (public/private) shape becomes one job; the + private-repo fallback note now fires from a live capability check instead + of an assumption, so it no longer misclassifies a private+GHAS-enabled + repository as unsupported, or a public+Dependency-Graph-disabled repository + as supported. +- argos's `unpinned @v4` and `newsdom-api`'s slightly older checkout pin are + both upgraded to the same current, verified pins the reusable workflow + uses, closing that drift too. + +See `docs/doctoring/dependency-review-reusable-workflow-consolidation.md` for +the full per-repo audit and the exact diffs each caller received. diff --git a/docs/doctoring/dependency-review-reusable-workflow-consolidation.md b/docs/doctoring/dependency-review-reusable-workflow-consolidation.md new file mode 100644 index 0000000000..7b63da83a6 --- /dev/null +++ b/docs/doctoring/dependency-review-reusable-workflow-consolidation.md @@ -0,0 +1,163 @@ +# Dependency Review reusable workflow consolidation + +## Decision + +`argos`, `mightyETL`, `newsdom-api`, and `scopeweave` each carried an +independently hand-written `.github/workflows/dependency-review.yml` running +`actions/dependency-review-action` on pull requests. All four are replaced by +one new reusable workflow, `.github/workflows/dependency-review.yml` in this +repository, plus a thin `workflow_call` caller left in place of each +repository's own file. See +[ADR-0024](../adr/0024-dependency-review-reusable-workflow-consolidation.md). + +## Field-by-field audit + +Reading all four files' full bodies (not just the job name and action used) +found real, repo-specific policy differences, not accidental copy drift: + +| Field | argos | mightyETL | newsdom-api | scopeweave | +| --- | --- | --- | --- | --- | +| `fail-on-severity` | `moderate` | `high` | unset → action default `low` | `moderate` | +| `allow-ghsas` | none | none | `GHSA-69w3-r845-3855` | none | +| `comment-summary-in-pr` | unset | unset | unset | `on-failure` | +| step `continue-on-error` | `true` | unset (blocking) | unset (blocking) | unset (blocking) | +| availability handling | none | static `repository.private` branch to a separate no-op job | none | dynamic `dependency-graph/compare` HTTP-status preflight: 200 → run, 403/404 → warn+skip, other → hard-fail | +| trigger | `pull_request: branches: [main, developmental]` | `pull_request` | `pull_request` | `pull_request`, `workflow_dispatch` | +| concurrency group | none | workflow+PR/ref group, cancel-in-progress | none | `dependency-review-`+PR/ref group, cancel-in-progress | +| `actions/checkout` pin | unpinned `@v4` | not used | SHA `3d3c42e5aac5ba805825da76410c181273ba90b1` | SHA `9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0` (v7.0.0) | +| `dependency-review-action` pin | unpinned `@v4` | SHA `a1d282b36b6f3519aa1f3fc636f609c47dddb294` (v5.0.0) | same SHA | same SHA | +| `FORCE_JAVASCRIPT_ACTIONS_TO_NODE24` | unset | unset | `true` | unset | + +Two decisions this audit drove (see ADR-0024 for the full reasoning): + +1. `fail_on_severity`, `allow_ghsas`, and `continue_on_error` stay per-caller + `workflow_call` inputs — flattening them to one shared value would + silently loosen mightyETL's `high` gate or newsdom-api's documented GHSA + allowlist exception. +2. scopeweave's dynamic Dependency Graph availability preflight (an actual + API capability check) replaces mightyETL's static + `github.event.repository.private` assumption everywhere, because the + assumption is provably wrong in both directions (a private+GHAS repo, or + a public+Dependency-Graph-disabled repo). argos and newsdom-api gain this + safety net for free; they previously had none. +3. `FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: true` (newsdom-api's original only) + is applied uniformly in the reusable workflow's job `env` rather than + made an input — it opts the job's JS actions (`checkout`, + `dependency-review-action`, present in all four originals) into GitHub's + Node 24 actions runtime ahead of the default cutover, which is a + forward-compatibility setting all four repositories benefit from + identically, not a per-repo policy choice. + +## Mechanism + +`.github/workflows/dependency-review.yml` (this repository) takes three +`workflow_call` inputs (`fail_on_severity`, `allow_ghsas`, +`continue_on_error`) and always runs the checkout → availability-preflight → +conditional dependency-review → conditional unavailability-note sequence. +Each calling repository's own thin `.github/workflows/dependency-review.yml` +keeps that repository's original `on:` trigger block (argos keeps its +`branches: [main, developmental]` restriction — a `workflow_call` target +cannot itself be what GitHub triggers on pull_request), gains a +`concurrency` block if it lacked one, and adds one job: +`uses: ContextualWisdomLab/.github/.github/workflows/dependency-review.yml@main` +with only that repository's non-default `with:` values. + +### argos caller + +```yaml +name: Dependency Review + +on: + pull_request: + branches: [main, developmental] + +concurrency: + group: dependency-review-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + +jobs: + dependency-review: + uses: ContextualWisdomLab/.github/.github/workflows/dependency-review.yml@main + with: + fail_on_severity: moderate + continue_on_error: true +``` + +### mightyETL caller + +```yaml +name: Dependency Review + +on: + pull_request: + +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + +jobs: + dependency-review: + uses: ContextualWisdomLab/.github/.github/workflows/dependency-review.yml@main + with: + fail_on_severity: high +``` + +### newsdom-api caller + +```yaml +name: dependency-review + +on: + pull_request: + +concurrency: + group: dependency-review-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + +jobs: + dependency-review: + uses: ContextualWisdomLab/.github/.github/workflows/dependency-review.yml@main + with: + fail_on_severity: low + allow_ghsas: "GHSA-69w3-r845-3855" +``` + +### scopeweave caller + +```yaml +name: Dependency Review + +on: + pull_request: + workflow_dispatch: + +concurrency: + group: dependency-review-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + +jobs: + dependency-review: + uses: ContextualWisdomLab/.github/.github/workflows/dependency-review.yml@main + with: + fail_on_severity: moderate +``` + +scopeweave's original supported a `workflow_dispatch` trigger, but its own +job never gated on the event at the job level — it always ran, and its +"Check dependency review support" step early-exited with `supported=false` +for any non-`pull_request` event (the availability check itself needs +`github.event.pull_request.base.sha` / `.head.sha`, which only exist on a +`pull_request` event). The reusable workflow's preflight step carries this +same event-name guard internally, so the caller does not need its own +job-level `if:` to reproduce it — `workflow_dispatch` stays in the trigger +list and the job still runs, harmlessly skipping the gate exactly as the +original did. + +## Verified before merge + +- `python3 -c "import yaml; yaml.safe_load(open(...))"` on all five files + (the reusable workflow and four callers). +- `actionlint` clean on all five files. +- Full `coverage run -m pytest tests` (2626 passed, 1 skipped) plus + `interrogate` on `ContextualWisdomLab/.github`, confirming the new + contract test and no regression elsewhere. diff --git a/tests/test_dependency_review_reusable_workflow_contract.py b/tests/test_dependency_review_reusable_workflow_contract.py new file mode 100644 index 0000000000..3a856b4e2b --- /dev/null +++ b/tests/test_dependency_review_reusable_workflow_contract.py @@ -0,0 +1,119 @@ +"""Contract for the reusable Dependency Review workflow. + +Replaces argos's, mightyETL's, newsdom-api's, and scopeweave's +independently hand-written ``dependency-review.yml`` files with one reusable +``workflow_call`` workflow, ``.github/workflows/dependency-review.yml``, plus +a thin caller left in each product repository. See +``docs/doctoring/dependency-review-reusable-workflow-consolidation.md`` and +``docs/adr/0024-dependency-review-reusable-workflow-consolidation.md`` for +why. +""" + +from __future__ import annotations + +from pathlib import Path + +_WORKFLOW = Path(".github/workflows/dependency-review.yml") + +_CHECKOUT_PIN = "9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0" +_DEPENDENCY_REVIEW_PIN = "a1d282b36b6f3519aa1f3fc636f609c47dddb294" + + +def _workflow_text() -> str: + """Read the reusable Dependency Review workflow as UTF-8 text.""" + return _WORKFLOW.read_text(encoding="utf-8") + + +def test_declares_workflow_call_with_three_inputs_and_recorded_defaults() -> None: + """Every genuinely-varying field found while auditing the four originals is an input.""" + workflow = _workflow_text() + assert "on:\n workflow_call:\n inputs:" in workflow + for name in ("fail_on_severity:", "allow_ghsas:", "continue_on_error:"): + assert name in workflow + + assert 'default: "moderate"' in workflow + assert 'default: ""' in workflow + assert "default: false" in workflow + + +def test_step_order_is_checkout_then_preflight_then_gated_steps() -> None: + """checkout -> dependency-graph preflight -> conditional gate/note, in that order.""" + workflow = _workflow_text() + order = [ + "actions/checkout@", + "Check dependency graph availability", + "Dependency review", + "Dependency graph unavailable note", + ] + positions = [workflow.index(marker) for marker in order] + assert positions == sorted(positions), "steps are out of order" + + +def test_dependency_review_and_note_steps_are_mutually_exclusive_on_availability() -> None: + """The gate and the fallback note must never both run.""" + workflow = _workflow_text() + assert ( + "if: steps.dependency_graph.outputs.available == 'true'\n" + " continue-on-error: ${{ inputs.continue_on_error }}" + in workflow + ) + assert "if: steps.dependency_graph.outputs.available != 'true'" in workflow + + +def test_inputs_are_forwarded_to_the_dependency_review_action() -> None: + """fail_on_severity and allow_ghsas must reach the underlying action untouched.""" + workflow = _workflow_text() + assert "fail-on-severity: ${{ inputs.fail_on_severity }}" in workflow + assert "allow-ghsas: ${{ inputs.allow_ghsas }}" in workflow + + +def test_action_pins_are_current_and_uniform() -> None: + """checkout and dependency-review-action share one current pin, not per-caller drift.""" + workflow = _workflow_text() + assert f"actions/checkout@{_CHECKOUT_PIN}" in workflow + assert ( + f"actions/dependency-review-action@{_DEPENDENCY_REVIEW_PIN}" in workflow + ) + + +def test_uniform_fields_are_hardcoded_not_parameterized() -> None: + """Fields byte-identical across all four originals stay static, not inputs.""" + workflow = _workflow_text() + assert "permissions:\n contents: read\n pull-requests: read" in workflow + assert "persist-credentials: false" in workflow + + +def test_forces_node24_runtime_for_js_actions() -> None: + """newsdom-api's Node24 opt-in applies uniformly, not only to that one caller.""" + workflow = _workflow_text() + assert "FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: true" in workflow + + +def test_availability_check_uses_the_dependency_graph_compare_api() -> None: + """The preflight must query the real capability, not infer from repository visibility.""" + workflow = _workflow_text() + assert "dependency-graph/compare" in workflow + assert "github.event.repository.private" not in workflow + + +def test_availability_check_distinguishes_unavailable_from_genuine_failure() -> None: + """403/404 means 'unavailable, skip gracefully'; any other status must hard-fail + the job instead of silently treating a real error the same as unavailability.""" + workflow = _workflow_text() + assert 'if [ "$status" = "403" ] || [ "$status" = "404" ]' in workflow + assert "available=false" in workflow + assert "::error::Dependency graph availability check failed with HTTP" in workflow + assert "exit 1" in workflow + + +def test_availability_check_only_runs_the_gate_for_pull_request_events() -> None: + """A non-pull_request trigger (e.g. workflow_dispatch) must skip the gate, not error, + since base/head SHAs only exist on a pull_request event.""" + workflow = _workflow_text() + assert '"${{ github.event_name }}" != "pull_request"' in workflow + + +def test_dependency_review_posts_a_pr_comment_on_failure() -> None: + """scopeweave's PR-comment-on-failure UX applies uniformly, not only to that one caller.""" + workflow = _workflow_text() + assert "comment-summary-in-pr: on-failure" in workflow