Skip to content

fix(ci): make the Dependency Review check able to fail again - #42537

Merged
rusackas merged 1 commit into
apache:masterfrom
kobihikri:ci/dependency-review-blocking
Jul 28, 2026
Merged

fix(ci): make the Dependency Review check able to fail again#42537
rusackas merged 1 commit into
apache:masterfrom
kobihikri:ci/dependency-review-blocking

Conversation

@kobihikri

Copy link
Copy Markdown
Contributor

SUMMARY

.github/workflows/dependency-review.yml sets continue-on-error: true on the actions/dependency-review-action step, so the step is reported as successful whatever the action finds, and the dependency-review job passes either way.

That step carries two policies:

  • fail-on-severity: critical — the vulnerability threshold
  • deny-licenses: MS-LPL, BUSL-1.1, QPL-1.0, Sleepycat, SSPL-1.0, CPOL-1.02, AGPL-3.0, GPL-1.0+, BSD-4-Clause-UC, NPL-1.0, NPL-1.1, JSON — which the comment directly above ties to https://www.apache.org/legal/resolved.html

With that flag in place, neither can stop a pull request. The file's own header comment says "if the workflow run is marked as required, PRs introducing known-vulnerable packages will be blocked from merging" — that is the behaviour the flag prevents.

I don't think it was deliberate. The line arrived in #31961 ("refactor: Upgrade to React 17"), whose description doesn't mention it, and there's no comment explaining it. My guess is it unblocked that one upgrade and then stayed. It's been there since January 2025.

This PR deletes that single line and changes nothing else.

TESTING INSTRUCTIONS

The change is one deletion in a workflow file, so the check on this PR is itself the test: Dependency Review should still run and pass here, since this PR adds no dependencies.

To see the behaviour it restores, open a scratch PR adding a dependency under one of the denied licences (an AGPL-3.0 package, say). With the line removed the job fails; with it present the job succeeds and only prints a warning.

One thing worth checking before merging: if master currently carries a dependency that trips fail-on-severity: critical or the licence denylist, this will start failing PRs until that is resolved. I could not verify that from outside the repo, so it seems worth a look — and if it does surface something, I'm glad to help with the follow-up.

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
  • Introduces new feature or API
  • Removes existing feature or API

To be clear about what this is: a CI configuration fix, not a security-vulnerability report. It makes no claim against the role and capability matrix in SECURITY.md and assumes no attacker principal, so per the automated-tooling requirements in AGENTS.md I've raised it as an ordinary PR rather than as a finding. If you'd still rather have it as a question first, tell me and I'll move it.

Disclosure: I used AI assistance to help spot this and draft the PR, and I verified the file, its history, and the surrounding behaviour myself.

continue-on-error: true on the dependency-review-action step meant neither
fail-on-severity: critical nor the ASF licence denylist could stop a PR.
The line arrived incidentally in apache#31961 (React 17 upgrade) with no stated
reason.
@dosubot dosubot Bot added the github_actions Pull requests that update GitHub Actions code label Jul 28, 2026
@bito-code-review

bito-code-review Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Bito Automatic Review Skipped - Files Excluded

Bito didn't auto-review this change because all changed files are in the exclusion list for automatic reviews. No action is needed if you didn't intend for the agent to review it. Otherwise, to manually trigger a review, type /review in a comment and save.
You can change the excluded files settings here, or contact your Bito workspace admin at evan@preset.io.

@rusackas rusackas added the merge-if-green If approved and tests are green, please go ahead and merge it for me label Jul 28, 2026
@rusackas
rusackas merged commit 22ad3f2 into apache:master Jul 28, 2026
61 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

github_actions Pull requests that update GitHub Actions code merge-if-green If approved and tests are green, please go ahead and merge it for me size/XS

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants