Skip to content

ci: restrict Dependabot auto-merge to direct devDependencies - #208

Merged
StephenTangCook merged 1 commit into
mainfrom
claude/nice-gates-7bf2hj
Sep 3, 2026
Merged

ci: restrict Dependabot auto-merge to direct devDependencies#208
StephenTangCook merged 1 commit into
mainfrom
claude/nice-gates-7bf2hj

Conversation

@StephenTangCook

@StephenTangCook StephenTangCook commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Summary

The Dependabot auto-merge workflow now only auto-approves and auto-merges PRs whose updated packages are all direct devDependencies. Production dependencies, GitHub Actions bumps, and lockfile-only updates are held for a maintainer, and the "held for manual review" comment states which rule applied.

Why

dependabot-automerge.yml approved and merged every non-major Dependabot PR with no dependency-type filter and no reviewer. Production dependencies were included. Because release-please.yml publishes main to npm, a compromised minor or patch release of any production dependency could reach every consumer of @tightknitai/block-kitchen with nobody having read the diff. The existing exclusion list only covered a handful of renderer/validator packages, so it mitigated the worst cases but not the general one.

A compromised devDependency reaches CI and developer machines at worst, so those keep auto-merging (unless on the exclusion list).

What changed

  • Single policy step. The exclusion-list step is folded into an Evaluate auto-merge policy step that emits automerge and hold-reason outputs. A PR is eligible only when it is not a major update, dependency-type is direct:development, and no package is on the exclusion list. Anything else, including an empty or unknown dependency-type, fails closed.
  • Exclusion list kept as defense in depth for packages that render or sanitize user content even when they appear as devDependencies (slack-web-api-client is a devDependency at the root today).
  • Held comment names the rule (major update / dependency type / exclusion list) so a maintainer can tell a policy hold from a CI failure. Majors now get this comment too; previously they were silently skipped.
  • No expression interpolation in run: scripts. Metadata reaches the comment body through env: instead of ${{ }} inside the shell script.
  • SECURITY-REVIEW.md F-006 gets a dated addendum describing the tightened gate.

Behavioral consequences worth knowing

  • Grouped PRs: dependabot/fetch-metadata reports the most sensitive dependency-type present (direct:production > direct:development > indirect), so a mixed group is held. This repo's dependabot.yml already groups by dependency type, so groups are homogeneous in practice.
  • Demo app: its runtime deps (react, react-dom, slack-web-api-client) are reported as production and will now be held. The demo is deployed publicly, so that seems right, but it is more manual work than before.
  • GitHub Actions: Dependabot classifies every Actions bump as production. Actions are major-tag pinned today, so Dependabot only opens major PRs for them and those were already held. If the actions are later SHA-pinned, minor/patch SHA bumps will be held too, which is the right posture for jobs that carry the npm publish token.
  • indirect updates (transitive security bumps in the lockfile) are held because Dependabot cannot say whether they reach the published package.

Test plan

  • actionlint on the workflow: no findings.
  • Ran the policy step's script locally across every input case and checked the outputs: dev patch (auto-merge), grouped dev (auto-merge), dev but on exclusion list (held), grouped dev containing an excluded package (held), production patch and minor (held), indirect (held), major dev and major prod (held), Actions bump (held), empty metadata (held).
  • pnpm typecheck / pnpm lint / pnpm test not run: no TypeScript or source files changed.
  • Next Dependabot PR on main after merge: confirm a devDependency bump auto-merges and a production bump gets the held comment.

Notes for reviewer

The suggested one-line fix of dependency-type == 'development' would never match: fetch-metadata's actual output values are direct:production, direct:development, and indirect. The gate here uses direct:development.

🤖 Generated with Claude Code

https://claude.ai/code/session_0155aTZRVhq6WYcU4B6u1Tqq


Generated by Claude Code


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

The auto-merge workflow approved and merged every non-major Dependabot
PR, including production dependencies, with nobody reading the diff.
release-please then publishes main to npm, so a compromised minor or
patch release of a production dependency could reach every consumer
unreviewed. The existing exclusion list only covered a handful of
renderer/validator packages.

Gate auto-merge on `dependency-type == direct:development` in addition
to the semver and exclusion-list checks. Production dependencies,
GitHub Actions bumps (reported as production by Dependabot), and
lockfile-only `indirect` updates are now held for a maintainer, and the
"held for manual review" comment states which rule applied. The
exclusion list stays as defense in depth for packages that render user
content even when they appear as devDependencies.

The policy is evaluated in a single step that emits `automerge` and
`hold-reason` outputs, and the comment body now receives metadata via
environment variables instead of interpolating expressions into the
shell script.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0155aTZRVhq6WYcU4B6u1Tqq
@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Cloudflare preview

✅ Deployed 2c7fe5c · Branch preview · Commit preview · Build logs

Copy link
Copy Markdown
Contributor Author

CI: Audit (pnpm) is red, and it is not this PR's failure.

The Audit root step fails on four high advisories against fast-uri@3.1.5, a transitive dependency reached via @tightknitai/slack-block-kit-validator > ajv > fast-uri:

Why it is not this PR's:

  • This PR changes only .github/workflows/dependabot-automerge.yml and SECURITY-REVIEW.md. pnpm-lock.yaml is byte-identical to main.
  • All four advisories were published on 2026-09-02, after the last CI run on main (2026-09-01, green). The same lockfile on main will fail the next time the audit job runs there, and every other open PR (ci: pin actions to commit SHAs and the publish npm CLI to an exact version #207 included) will hit it too.
  • It reproduces deterministically: pnpm audit --audit-level=high on an unchanged checkout of main reports the same 4 highs locally, so a re-run would not change the outcome.

No open PR fixes it yet. The fix is a lockfile-only bump and needs no package.json change, because ajv's range (^3.0.1) already admits the patched version:

pnpm update fast-uri --lockfile-only
pnpm audit --audit-level=high   # → No known vulnerabilities found

I ran exactly that locally: it changes 4 lines in pnpm-lock.yaml (fast-uri@3.1.53.1.7) and the audit passes. I have not pushed it here because it is unrelated to the auto-merge policy change; it belongs in its own PR (or a Dependabot security PR, which under this workflow would be held as indirect for a human to merge, which is the intended behavior). Once that lands on main, this PR only needs the base merged in to go green.


Generated by Claude Code

@StephenTangCook
StephenTangCook merged commit acc97af into main Sep 3, 2026
13 of 14 checks passed
@StephenTangCook
StephenTangCook deleted the claude/nice-gates-7bf2hj branch September 3, 2026 21:53
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.

2 participants