Skip to content

feat(workflows): add harden-runner and comment_summary_in_pr input for naruon #1732

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
seonghobae merged 1 commit into from
Sep 2, 2026
Merged

feat(workflows): add harden-runner and comment_summary_in_pr input for naruon #1732

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 20 additions & 4 deletions .github/workflows/dependency-review.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Reusable Dependency Review (workflow_call), consolidating the four
# near-identical dependency-review.yml files argos, mightyETL, newsdom-api,
# and scopeweave each carried independently. See
# Reusable Dependency Review (workflow_call), consolidating the near-
# identical dependency-review.yml files argos, mightyETL, newsdom-api,
# scopeweave, and naruon 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.
Expand Down Expand Up @@ -68,6 +68,17 @@ on:
required: false
type: boolean
default: false
comment_summary_in_pr:
description: >-
Value forwarded to dependency-review-action's comment-summary-in-pr
input. Default "on-failure" (scopeweave's original choice, applied
uniformly when this input was still hardcoded); naruon explicitly
opts out with "never" -- an explicit per-repo choice, not
accidental drift, so it must stay an input rather than being
flattened to one value.
required: false
type: string
default: "on-failure"

permissions:
contents: read
Expand All @@ -83,6 +94,11 @@ jobs:
# since it is a forward-compatibility setting, not a per-repo policy.
FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: true
steps:
- name: Harden the runner (Audit all outbound calls)
uses: step-security/harden-runner@b09bb98e06d4d774595224525879c09bc6e98c40 # v2.20.1
with:
egress-policy: audit

- name: Checkout
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
with:
Expand Down Expand Up @@ -138,7 +154,7 @@ jobs:
with:
fail-on-severity: ${{ inputs.fail_on_severity }}
allow-ghsas: ${{ inputs.allow_ghsas }}
comment-summary-in-pr: on-failure
comment-summary-in-pr: ${{ inputs.comment_summary_in_pr }}

@devin-ai-integration devin-ai-integration Bot Sep 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 PR failure summaries are never posted

With comment_summary_in_pr set to on-failure or always, commentPr receives a read-only token. Comment creation returns 403 and only logs a warning.

Prompt for agents 
The reusable dependency-review workflow forwards comment_summary_in_pr values that enable PR comments, but its workflow-level permissions grant only pull-requests: read. actions/dependency-review-action requires pull-requests: write for on-failure or always and catches a 403 as a warning, so the job still completes without the promised comment. Update the reusable workflow permission and its contract test to allow writing when comment summaries are enabled. Also ensure every thin caller grants the reusable job sufficient pull-request permission, because a called workflow cannot elevate permissions beyond its caller. Preserve naruon's explicit never behavior and account for GitHub's reduced permissions on fork pull requests.
Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.


- name: Dependency graph unavailable note
if: steps.dependency_graph.outputs.available != 'true' && github.event_name == 'pull_request'
Expand Down
65 changes: 52 additions & 13 deletions docs/adr/0024-dependency-review-reusable-workflow-consolidation.md
View file
Open in desktop

@devin-ai-integration devin-ai-integration Bot Sep 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 ADR decision remains outdated

The Decision still defines three inputs and four callers. The later addendum contradicts this authoritative summary, leaving two active designs documented.

(Refers to this code)

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
- **Status:** Accepted
- **Date:** 2026-09-02
- **Scope:** `.github/workflows/dependency-review.yml` (new, central, `workflow_call`);
thin callers in `argos`, `mightyETL`, `newsdom-api`, `scopeweave`
thin callers in `argos`, `mightyETL`, `newsdom-api`, `scopeweave`, `naruon`
(`naruon` added same-day, see "Addendum: naruon" below)

## Context

Expand All @@ -17,18 +18,19 @@ 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 |
| Field | argos | mightyETL | newsdom-api | scopeweave | naruon |
| --- | --- | --- | --- | --- | --- |
| `fail-on-severity` | `moderate` | `high` | unset (action default `low`) | `moderate` | `moderate` |
| `allow-ghsas` | none | none | `GHSA-69w3-r845-3855` | none | none |
| `comment-summary-in-pr` | unset | unset | unset | `on-failure` | `never` (explicit) |
| step-level `continue-on-error` | `true` | unset (blocking) | 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 | none |
| `step-security/harden-runner` | absent | absent | absent | absent | present (egress audit) |
| trigger scope | `pull_request: branches: [main, developmental]` | `pull_request` (all branches) | `pull_request` (all branches) | `pull_request` + `workflow_dispatch` | `pull_request: branches: [develop, master, release/**]` + `workflow_dispatch` |
| concurrency group | none | `${{ github.workflow }}-${{ github.event.pull_request.number \|\| github.ref }}` | none | `dependency-review-${{ github.event.pull_request.number \|\| github.ref }}` | `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) | SHA `3d3c42e5...` (v7.0.1) |
| `dependency-review-action` pin | unpinned `@v4` | SHA `a1d282b3...` (v5.0.0) | SHA `a1d282b3...` | SHA `a1d282b3...` | SHA `a1d282b3...` |
| `FORCE_JAVASCRIPT_ACTIONS_TO_NODE24` | unset | unset | `true` | unset | unset |

Two findings changed the design from a naive copy-paste consolidation:

Expand Down Expand Up @@ -126,3 +128,40 @@ unreviewed against every caller's PR checks; (2) converting a job to
`<caller job> / <called job>` name, which broke `newsdom-api`'s branch
protection (it required the old standalone name) until that required-check
name was updated to match.

## Addendum: naruon (2026-09-02, later the same day)

A peer session's fresh org-wide workflow-duplication survey (63 repos, 255
workflow files) found a fifth repository, `naruon`, independently carrying
its own `dependency-review.yml` — missed by the original survey this ADR's
consolidation was based on, which never covered `naruon`. Auditing it found
two real, non-cosmetic differences from the four originals above:

1. **A `step-security/harden-runner` step (egress audit), present in none
of the original four.** Not a per-repo policy — it is a uniformly
beneficial security-hardening practice already standard elsewhere in
this org's own workflows (e.g. `pr-review-autofix.yml`), so it is added
to the reusable workflow itself, as its first step, applying to every
caller including the four already migrated (no caller-side change
needed for this one).
2. **`comment-summary-in-pr: never`, an explicit opt-out**, conflicting
with the earlier decision (see item 3 above) to hardcode
`comment-summary-in-pr: on-failure` uniformly for every caller. That
earlier decision was made when only scopeweave's original set the
field at all, so "hardcode it uniformly" cost no caller its own choice.
naruon proves that assumption wrong: hardcoding it now would silently
overturn an explicit, deliberate choice naruon's original workflow
made. Corrected by making `comment_summary_in_pr` a proper
`workflow_call` input (default `"on-failure"`, preserving current
behavior for the four already-migrated callers with no changes needed
on their side; `naruon`'s caller explicitly sets `"never"`).

`naruon`'s other fields (`fail-on-severity: moderate`, no `allow-ghsas`,
multi-branch trigger `develop`/`master`/`release/**` plus
`workflow_dispatch`, its own `concurrency` group, job-level `permissions:`
redundant with the workflow-level block, and an informational "Log
dependency review policy" step) either match an existing input, are
caller-side triggers/concurrency untouched by this ADR's design, or (the
informational logging step, and the redundant job-level `permissions:`)
are dropped as they add no policy value the central workflow or the
underlying action doesn't already provide.
93 changes: 78 additions & 15 deletions docs/doctoring/dependency-review-reusable-workflow-consolidation.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,25 @@ repository's own file. See
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 |
| Field | argos | mightyETL | newsdom-api | scopeweave | naruon |
| --- | --- | --- | --- | --- | --- |
| `fail-on-severity` | `moderate` | `high` | unset → action default `low` | `moderate` | `moderate` |
| `allow-ghsas` | none | none | `GHSA-69w3-r845-3855` | none | none |
| `comment-summary-in-pr` | unset | unset | unset | `on-failure` | `never` (explicit) |
| step `continue-on-error` | `true` | unset (blocking) | 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 | none |
| `harden-runner` (egress audit) | absent | absent | absent | absent | present |
| trigger | `pull_request: branches: [main, developmental]` | `pull_request` | `pull_request` | `pull_request`, `workflow_dispatch` | `pull_request: branches: [develop, master, release/**]`, `workflow_dispatch` |
| concurrency group | none | workflow+PR/ref group, cancel-in-progress | none | `dependency-review-`+PR/ref group, cancel-in-progress | `dependency-review-`+PR/ref group, cancel-in-progress |
| `actions/checkout` pin | unpinned `@v4` | not used | SHA `3d3c42e5aac5ba805825da76410c181273ba90b1` | SHA `9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0` (v7.0.0) | SHA `3d3c42e5aac5ba805825da76410c181273ba90b1` (v7.0.1) |
| `dependency-review-action` pin | unpinned `@v4` | SHA `a1d282b36b6f3519aa1f3fc636f609c47dddb294` (v5.0.0) | same SHA | same SHA | same SHA |
| `FORCE_JAVASCRIPT_ACTIONS_TO_NODE24` | unset | unset | `true` | unset | unset |

naruon was found later the same day by a peer session's fresh org-wide survey
-- missed by the original 4-repo survey this consolidation started from. See
"Addendum: naruon" below for the two real design changes it required
(`comment_summary_in_pr` becoming an input instead of a hardcoded uniform
value, and adding `harden-runner` uniformly).

Two decisions this audit drove (see ADR-0024 for the full reasoning):

Expand All @@ -50,10 +57,11 @@ Two decisions this audit drove (see ADR-0024 for the full reasoning):

## Mechanism

`.github/workflows/dependency-review.yml` (this repository) takes three
`.github/workflows/dependency-review.yml` (this repository) takes four
`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.
`continue_on_error`, `comment_summary_in_pr`) and always runs the
harden-runner → 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
Expand Down Expand Up @@ -153,6 +161,61 @@ 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.

### naruon caller

```yaml
name: Dependency Review

on:
pull_request:
branches:
- develop
- master
- "release/**"
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@<commit-sha>
with:
fail_on_severity: moderate
comment_summary_in_pr: never
```

naruon's original also had a job-level `permissions:` block duplicating the
workflow-level one, and an informational "Log dependency review policy" step
that only printed the policy text and base/head refs -- neither is carried
into the caller: the job-level `permissions:` was redundant, and the log
step added no policy value beyond what `actions/dependency-review-action`
itself already reports on failure.

## Addendum: naruon (2026-09-02, later the same day)

A peer session's fresh org-wide workflow-duplication survey (63 repos, 255
workflow files) found `naruon` independently carrying its own
`dependency-review.yml` -- missed by the original 4-repo survey. Auditing it
found two real differences, not cosmetic ones:

1. **`step-security/harden-runner` (egress audit)**, absent from all four
original callers. Not a per-repo policy choice -- a uniformly beneficial
hardening practice already standard elsewhere in this org (e.g.
`pr-review-autofix.yml`). Added to the reusable workflow itself as its
first step, so every caller (the four already migrated included) gets it
with no caller-side change required.
2. **`comment-summary-in-pr: never`**, an explicit opt-out that directly
conflicts with the earlier decision to hardcode
`comment-summary-in-pr: on-failure` uniformly (made when only scopeweave's
original set the field, so hardcoding it cost no caller its own choice).
Silently applying that hardcoded value to naruon would overturn a
deliberate choice its original workflow made. Fixed by making
`comment_summary_in_pr` a proper `workflow_call` input, default
`"on-failure"` (no change for the four already-migrated callers),
`naruon`'s caller explicitly setting `"never"`.

## Post-merge corrections (2026-09-02, same day)

Two real problems surfaced after the four caller PRs opened, both caught
Expand Down
41 changes: 32 additions & 9 deletions tests/test_dependency_review_reusable_workflow_contract.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,22 +24,29 @@ def _workflow_text() -> str:
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."""
def test_declares_workflow_call_with_four_inputs_and_recorded_defaults() -> None:
"""Every genuinely-varying field found while auditing the five 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:"):
for name in (
"fail_on_severity:",
"allow_ghsas:",
"continue_on_error:",
"comment_summary_in_pr:",
):
assert name in workflow

assert 'default: "moderate"' in workflow
assert 'default: ""' in workflow
assert "default: false" in workflow
assert 'default: "on-failure"' in workflow


def test_step_order_is_checkout_then_preflight_then_gated_steps() -> None:
"""checkout -> dependency-graph preflight -> conditional gate/note, in that order."""
def test_step_order_is_harden_then_checkout_then_preflight_then_gated_steps() -> None:
"""harden-runner -> checkout -> dependency-graph preflight -> conditional gate/note."""
workflow = _workflow_text()
order = [
"Harden the runner",
"actions/checkout@",
"Check dependency graph availability",
"Dependency review",
Expand All @@ -61,10 +68,18 @@ def test_dependency_review_and_note_steps_are_mutually_exclusive_on_availability


def test_inputs_are_forwarded_to_the_dependency_review_action() -> None:
"""fail_on_severity and allow_ghsas must reach the underlying action untouched."""
"""fail_on_severity, allow_ghsas, and comment_summary_in_pr must reach the action untouched."""
workflow = _workflow_text()
assert "fail-on-severity: ${{ inputs.fail_on_severity }}" in workflow
assert "allow-ghsas: ${{ inputs.allow_ghsas }}" in workflow
assert "comment-summary-in-pr: ${{ inputs.comment_summary_in_pr }}" in workflow


def test_harden_runner_audits_egress() -> None:
"""naruon's harden-runner step applies uniformly, not only to that one caller."""
workflow = _workflow_text()
assert "step-security/harden-runner@" in workflow
assert "egress-policy: audit" in workflow


def test_action_pins_are_current_and_uniform() -> None:
Expand Down Expand Up @@ -113,7 +128,15 @@ def test_availability_check_only_runs_the_gate_for_pull_request_events() -> None
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."""
def test_dependency_review_comment_summary_defaults_to_on_failure() -> None:
"""scopeweave's PR-comment-on-failure UX applies uniformly by default, overridable per caller.

naruon explicitly overrides it to "never" -- see
test_declares_workflow_call_with_four_inputs_and_recorded_defaults for
the default assertion and test_inputs_are_forwarded_to_the_dependency_review_action
for the forwarding assertion; this test just pins the specific default
value chosen (scopeweave's original, not naruon's or some other value).
"""
workflow = _workflow_text()
assert "comment-summary-in-pr: on-failure" in workflow
assert 'comment_summary_in_pr:\n' in workflow
assert 'default: "on-failure"' in workflow
Loading