Skip to content

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

Merged
seonghobae merged 1 commit into
mainfrom
feat/dependency-review-harden-runner-and-naruon
Sep 2, 2026
Merged

feat(workflows): add harden-runner and comment_summary_in_pr input for naruon#1732
seonghobae merged 1 commit into
mainfrom
feat/dependency-review-harden-runner-and-naruon

Conversation

@seonghobae

@seonghobae seonghobae commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Summary

  • A peer session's 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 this consolidation was based on.
  • Two real, non-cosmetic differences from the four already-migrated callers:
    1. step-security/harden-runner (egress audit) — absent from all four originals, added to the reusable workflow itself uniformly (a hardening practice already standard elsewhere in this org, not a per-repo policy).
    2. comment-summary-in-pr: never — an explicit opt-out that conflicts with the earlier decision to hardcode on-failure uniformly. Fixed by making comment_summary_in_pr a proper input (default "on-failure", no behavior change for the four already-migrated callers).
  • naruon's own caller PR follows separately once this merges (needs the final commit SHA to pin to).

Test plan

  • python3 -c "import yaml; yaml.safe_load(...)" on the edited workflow
  • actionlint .github/workflows/dependency-review.yml
  • Full suite: coverage run -m pytest tests → 2639 passed, 1 skipped

🤖 Generated with Claude Code


Devin Review

…r naruon

A peer session's 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 this
consolidation was based on. Auditing it found two real, non-cosmetic
differences from the four already-migrated callers:

1. A step-security/harden-runner step (egress audit), present in none of
   the originals. Not a per-repo policy -- a uniformly beneficial hardening
   practice already standard elsewhere in this org. Added to the reusable
   workflow itself as its first step; the four already-migrated callers
   get it automatically, no caller-side change needed.
2. comment-summary-in-pr: never, an explicit opt-out that conflicts with
   the earlier decision to hardcode comment-summary-in-pr: on-failure
   uniformly (made when only scopeweave's original set the field at all).
   Silently applying that hardcoded value to naruon would overturn a
   deliberate choice its original workflow made -- the exact mistake this
   ADR already flagged for fail_on_severity/allow_ghsas. Fixed by making
   comment_summary_in_pr a proper workflow_call input (default
   "on-failure", no change for the four already-migrated callers).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Sep 2, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 43 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: 04c7480a-b881-4bb2-a957-e6a765377c3f

📥 Commits

Reviewing files that changed from the base of the PR and between 63bf498 and fd27213.

📒 Files selected for processing (4)
  • .github/workflows/dependency-review.yml
  • docs/adr/0024-dependency-review-reusable-workflow-consolidation.md
  • docs/doctoring/dependency-review-reusable-workflow-consolidation.md
  • tests/test_dependency_review_reusable_workflow_contract.py

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@devin-ai-integration devin-ai-integration Bot left a comment

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.

Devin Review found 2 potential issues.

Devin Review

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.

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 }}

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.

@seonghobae
seonghobae merged commit 5f8e5b2 into main Sep 2, 2026
8 of 26 checks passed
@seonghobae
seonghobae deleted the feat/dependency-review-harden-runner-and-naruon branch September 2, 2026 12:35
seonghobae added a commit to ContextualWisdomLab/mightyETL that referenced this pull request Sep 2, 2026
…_in_pr

A peer session's org-wide survey found naruon independently carrying its
own dependency-review.yml with a harden-runner egress-audit step this
consolidation's original four callers lacked. That step (and the
comment_summary_in_pr input naruon's explicit "never" choice required)
were added to the central reusable workflow in
ContextualWisdomLab/.github#1732, after this caller's original PR opened.
Re-pinning to the new commit picks up harden-runner for free before this
PR's first merge -- no `with:` change needed, since this repo never set
comment_summary_in_pr and the new input's default ("on-failure") matches
the value this reusable workflow already hardcoded before.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
seonghobae added a commit to ContextualWisdomLab/newsdom-api that referenced this pull request Sep 2, 2026
…_in_pr

A peer session's org-wide survey found naruon independently carrying its
own dependency-review.yml with a harden-runner egress-audit step this
consolidation's original four callers lacked. That step (and the
comment_summary_in_pr input naruon's explicit "never" choice required)
were added to the central reusable workflow in
ContextualWisdomLab/.github#1732, after this caller's original PR opened.
Re-pinning to the new commit picks up harden-runner for free before this
PR's first merge -- no `with:` change needed, since this repo never set
comment_summary_in_pr and the new input's default ("on-failure") matches
the value this reusable workflow already hardcoded before.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
seonghobae added a commit to ContextualWisdomLab/scopeweave that referenced this pull request Sep 2, 2026
…_in_pr

A peer session's org-wide survey found naruon independently carrying its
own dependency-review.yml with a harden-runner egress-audit step this
consolidation's original four callers lacked. That step (and the
comment_summary_in_pr input naruon's explicit "never" choice required)
were added to the central reusable workflow in
ContextualWisdomLab/.github#1732, after this caller's original PR opened.
Re-pinning to the new commit picks up harden-runner for free before this
PR's first merge -- no `with:` change needed, since this repo never set
comment_summary_in_pr and the new input's default ("on-failure") matches
the value this reusable workflow already hardcoded before.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
seonghobae added a commit to ContextualWisdomLab/argos that referenced this pull request Sep 2, 2026
…_in_pr (#559)

A peer session's org-wide survey found naruon independently carrying its
own dependency-review.yml with a harden-runner egress-audit step this
consolidation's original four callers lacked. That step was added to the
central reusable workflow in ContextualWisdomLab/.github#1732, after this
caller already merged. Re-pinning picks it up with no with: change needed.

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant