From 637276ad5d6190e165c6487e0e97551b38db7ab5 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 2 Sep 2026 19:47:50 +0900 Subject: [PATCH 1/3] feat(workflows): add reusable dependency-review.yml for 4 product repos argos, mightyETL, newsdom-api, and scopeweave each carried an independently hand-written dependency-review.yml. Auditing all four found real per-repo policy differences (fail-on-severity, an allow-ghsas exception, a non-blocking continue-on-error) that must stay per-caller inputs, plus one correctness bug: mightyETL's static repository.private check for Dependency Graph/GHAS availability is wrong in both directions. Consolidate into one workflow_call workflow that generalizes scopeweave's dynamic dependency-graph compare-API preflight (the one design that checks the actual capability instead of guessing from visibility) to all four callers. See docs/adr/0024-dependency-review-reusable-workflow-consolidation.md and docs/doctoring/dependency-review-reusable-workflow-consolidation.md for the full field audit and each caller's exact replacement content. Co-Authored-By: Claude Sonnet 5 --- .github/workflows/dependency-review.yml | 117 +++++++++++++ ...-review-reusable-workflow-consolidation.md | 95 +++++++++++ ...-review-reusable-workflow-consolidation.md | 154 ++++++++++++++++++ ...dency_review_reusable_workflow_contract.py | 90 ++++++++++ 4 files changed, 456 insertions(+) create mode 100644 .github/workflows/dependency-review.yml create mode 100644 docs/adr/0024-dependency-review-reusable-workflow-consolidation.md create mode 100644 docs/doctoring/dependency-review-reusable-workflow-consolidation.md create mode 100644 tests/test_dependency_review_reusable_workflow_contract.py diff --git a/.github/workflows/dependency-review.yml b/.github/workflows/dependency-review.yml new file mode 100644 index 0000000000..011484043b --- /dev/null +++ b/.github/workflows/dependency-review.yml @@ -0,0 +1,117 @@ +# 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 + 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 + + api_url="${GITHUB_API_URL:-https://api.github.com}" + status="$( + curl -fsS -o /dev/null -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" + else + 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 likely unavailable)." + echo "available=false" >>"$GITHUB_OUTPUT" + fi + + - 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 }} + + - name: Dependency graph unavailable note + if: steps.dependency_graph.outputs.available != 'true' + 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..3710039dc2 --- /dev/null +++ b/docs/adr/0024-dependency-review-reusable-workflow-consolidation.md @@ -0,0 +1,95 @@ +# 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`) | unset (action default `low`) | +| `allow-ghsas` | none | none | `GHSA-69w3-r845-3855` | none | +| 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), gates the action step, emits a warning note otherwise | +| 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...` | + +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). + +## 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..32fb1b9a27 --- /dev/null +++ b/docs/doctoring/dependency-review-reusable-workflow-consolidation.md @@ -0,0 +1,154 @@ +# 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` | unset → action default `low` | +| `allow-ghsas` | none | none | `GHSA-69w3-r845-3855` | none | +| 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 | +| 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 | + +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. + +## 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: + if: github.event_name == 'pull_request' + uses: ContextualWisdomLab/.github/.github/workflows/dependency-review.yml@main + with: + fail_on_severity: low +``` + +scopeweave's original also supported a `workflow_dispatch` trigger, but its +own dependency-review job only ever ran the gate for `pull_request` events +(the availability check itself early-exited with a "only runs for +pull_request events" note otherwise) — the caller keeps `workflow_dispatch` +in its trigger list for manual runs of other jobs in that repository's +workflow file, if any, but gates this job to `pull_request` to preserve +that exact original behavior; the reusable workflow's own preflight step +still requires `github.event.pull_request.base.sha` / `.head.sha`, which +only exist on a `pull_request` event. + +## 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..98bd28a120 --- /dev/null +++ b/tests/test_dependency_review_reusable_workflow_contract.py @@ -0,0 +1,90 @@ +"""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_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 From 9efca4700dc3bece2b2f996231a014cb96e59f4b Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 2 Sep 2026 19:52:50 +0900 Subject: [PATCH 2/3] fix(workflows): apply FORCE_JAVASCRIPT_ACTIONS_TO_NODE24 uniformly newsdom-api's original dependency-review.yml set this ahead of Node 20's actions-runtime EOL; the other three originals didn't. It's a forward-compatibility setting, not a per-repo policy, so bake it into the reusable workflow's job env for all four callers instead of dropping it for newsdom-api or leaving the other three without it. Co-Authored-By: Claude Sonnet 5 --- .github/workflows/dependency-review.yml | 6 ++++++ ...ependency-review-reusable-workflow-consolidation.md | 10 ++++++++++ ...ependency-review-reusable-workflow-consolidation.md | 8 ++++++++ ...est_dependency_review_reusable_workflow_contract.py | 6 ++++++ 4 files changed, 30 insertions(+) diff --git a/.github/workflows/dependency-review.yml b/.github/workflows/dependency-review.yml index 011484043b..691b3d441b 100644 --- a/.github/workflows/dependency-review.yml +++ b/.github/workflows/dependency-review.yml @@ -68,6 +68,12 @@ permissions: 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 diff --git a/docs/adr/0024-dependency-review-reusable-workflow-consolidation.md b/docs/adr/0024-dependency-review-reusable-workflow-consolidation.md index 3710039dc2..1c0f94a79a 100644 --- a/docs/adr/0024-dependency-review-reusable-workflow-consolidation.md +++ b/docs/adr/0024-dependency-review-reusable-workflow-consolidation.md @@ -27,6 +27,7 @@ A field-by-field audit of all four files (2026-09-02) found: | 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: @@ -48,6 +49,15 @@ Two findings changed the design from a naive copy-paste consolidation: 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). +3. **`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 diff --git a/docs/doctoring/dependency-review-reusable-workflow-consolidation.md b/docs/doctoring/dependency-review-reusable-workflow-consolidation.md index 32fb1b9a27..e823715c0d 100644 --- a/docs/doctoring/dependency-review-reusable-workflow-consolidation.md +++ b/docs/doctoring/dependency-review-reusable-workflow-consolidation.md @@ -25,6 +25,7 @@ found real, repo-specific policy differences, not accidental copy drift: | 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): @@ -38,6 +39,13 @@ Two decisions this audit drove (see ADR-0024 for the full reasoning): 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 diff --git a/tests/test_dependency_review_reusable_workflow_contract.py b/tests/test_dependency_review_reusable_workflow_contract.py index 98bd28a120..e0079b2d1a 100644 --- a/tests/test_dependency_review_reusable_workflow_contract.py +++ b/tests/test_dependency_review_reusable_workflow_contract.py @@ -83,6 +83,12 @@ def test_uniform_fields_are_hardcoded_not_parameterized() -> None: 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() From 2930850021073b4d8be5d3fcb59cc5c3b74a6f18 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 2 Sep 2026 19:57:30 +0900 Subject: [PATCH 3/3] fix(workflows): correct scopeweave audit, preserve its error handling Two mistakes in the initial audit, caught before merge by re-reading scopeweave's full original file rather than the truncated excerpt used earlier: 1. scopeweave's fail-on-severity is "moderate", not unset/action-default "low" as the first pass claimed -- fixed in the ADR, doctoring doc, and (separately, in the scopeweave caller PR) the caller's `with:` block. 2. scopeweave's availability preflight distinguishes a confirmed- unavailable response (403/404 -> warn and skip the gate) from any other unexpected HTTP status (-> hard-fail the job with the response body). The first draft of the reusable workflow collapsed this to "any non-200 means unavailable", which would silently skip the security gate on a real failure (auth problem, GitHub API outage) instead of surfacing it. Restored the original distinction, plus the pull_request-only event guard and comment-summary-in-pr: on-failure (also uniformly applied -- UX only, doesn't change pass/fail semantics) that the first draft dropped. 4 new contract tests pin the corrected behavior. Co-Authored-By: Claude Sonnet 5 --- .github/workflows/dependency-review.yml | 24 ++++++++++++++--- ...-review-reusable-workflow-consolidation.md | 23 +++++++++++++--- ...-review-reusable-workflow-consolidation.md | 27 ++++++++++--------- ...dency_review_reusable_workflow_contract.py | 23 ++++++++++++++++ 4 files changed, 76 insertions(+), 21 deletions(-) diff --git a/.github/workflows/dependency-review.yml b/.github/workflows/dependency-review.yml index 691b3d441b..d199dd36a0 100644 --- a/.github/workflows/dependency-review.yml +++ b/.github/workflows/dependency-review.yml @@ -91,9 +91,16 @@ jobs: 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 /dev/null -w '%{http_code}' \ + 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" \ @@ -103,11 +110,19 @@ jobs: if [ "$status" = "200" ]; then echo "available=true" >>"$GITHUB_OUTPUT" - else - 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 likely unavailable)." + 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 }} @@ -115,9 +130,10 @@ jobs: 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' + 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 index 1c0f94a79a..51bc203c37 100644 --- a/docs/adr/0024-dependency-review-reusable-workflow-consolidation.md +++ b/docs/adr/0024-dependency-review-reusable-workflow-consolidation.md @@ -19,10 +19,11 @@ 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`) | unset (action default `low`) | +| `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), gates the action step, emits a warning note otherwise | +| 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) | @@ -48,8 +49,22 @@ Two findings changed the design from a naive copy-paste consolidation: 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). -3. **`FORCE_JAVASCRIPT_ACTIONS_TO_NODE24` is a forward-compatibility setting, + 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`, diff --git a/docs/doctoring/dependency-review-reusable-workflow-consolidation.md b/docs/doctoring/dependency-review-reusable-workflow-consolidation.md index e823715c0d..7b63da83a6 100644 --- a/docs/doctoring/dependency-review-reusable-workflow-consolidation.md +++ b/docs/doctoring/dependency-review-reusable-workflow-consolidation.md @@ -17,10 +17,11 @@ 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` | unset → action default `low` | +| `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 | +| 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) | @@ -136,21 +137,21 @@ concurrency: jobs: dependency-review: - if: github.event_name == 'pull_request' uses: ContextualWisdomLab/.github/.github/workflows/dependency-review.yml@main with: - fail_on_severity: low + fail_on_severity: moderate ``` -scopeweave's original also supported a `workflow_dispatch` trigger, but its -own dependency-review job only ever ran the gate for `pull_request` events -(the availability check itself early-exited with a "only runs for -pull_request events" note otherwise) — the caller keeps `workflow_dispatch` -in its trigger list for manual runs of other jobs in that repository's -workflow file, if any, but gates this job to `pull_request` to preserve -that exact original behavior; the reusable workflow's own preflight step -still requires `github.event.pull_request.base.sha` / `.head.sha`, which -only exist on a `pull_request` event. +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 diff --git a/tests/test_dependency_review_reusable_workflow_contract.py b/tests/test_dependency_review_reusable_workflow_contract.py index e0079b2d1a..3a856b4e2b 100644 --- a/tests/test_dependency_review_reusable_workflow_contract.py +++ b/tests/test_dependency_review_reusable_workflow_contract.py @@ -94,3 +94,26 @@ def test_availability_check_uses_the_dependency_graph_compare_api() -> None: 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