-
Notifications
You must be signed in to change notification settings - Fork 0
feat(kodiak): auto-approve+merge Dependabot patch/minor bumps #6
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| # Merge-governance surface. Any change under /.github/ — including a NEW | ||
| # workflow file, which can run on its own PR with GITHUB_TOKEN write | ||
| # permissions — requires a human code-owner review to merge. Pairs with | ||
| # branch protection "Require review from Code Owners". | ||
| /.github/ @derivita/engineering | ||
|
|
||
| # Kodiak's merge-queue config — an unreviewed edit here could weaken or | ||
| # disable branch-protection-equivalent merge gating (e.g. the automerge | ||
| # label, blocking labels, or merge method). | ||
| /.kodiak.toml @derivita/engineering | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| version: 2 | ||
| updates: | ||
| - package-ecosystem: npm | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Non-blocking, and a scope call rather than a defect. With only A The two-lockfile question the description raises is already settled, incidentally — #4 and #5 each updated
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed this is sound and non-blocking. Since you've framed it as a scope call ("your call" whether it belongs in this PR or a follow-up) rather than a defect in what's here, I'm leaving that decision to a repo owner rather than making it unilaterally. Not resolving this thread so it stays visible for that call. |
||
| directory: / | ||
| schedule: | ||
| interval: weekly | ||
| cooldown: | ||
| default-days: 7 | ||
| groups: | ||
| minor-and-patch: | ||
| update-types: | ||
| - minor | ||
| - patch | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| name: Dependabot Auto-merge Label | ||
|
|
||
| # Labels Dependabot's own patch/minor npm bumps `automerge` + | ||
| # `dependabot-automerge` so Kodiak can auto-approve and merge them. See | ||
| # derivita/shared-workflows's dependabot-automerge-label.yml (the reusable | ||
| # workflow this calls) for the full security rationale, and | ||
| # derivita/infrastructure#1716 for the reference rollout. | ||
| on: | ||
| pull_request_target: | ||
| types: [opened, reopened, synchronize] | ||
| branches: [main] | ||
|
|
||
| permissions: | ||
| contents: read | ||
| pull-requests: write | ||
|
|
||
| jobs: | ||
| label: | ||
| # Skip the runner allocation entirely for human PRs — the callee no-ops | ||
| # on these anyway (it does its own author and head-commit-signature | ||
| # checks), but there's no reason to spend a job on every PR to main to | ||
| # find that out. | ||
| if: github.event.pull_request.user.login == 'dependabot[bot]' | ||
| # Pinned to the v1 tag's commit, not the mutable tag itself — this | ||
| # workflow controls what gets auto-approved, and anyone with write | ||
| # access to shared-workflows can retarget a tag with no PR in this repo. | ||
| uses: derivita/shared-workflows/.github/workflows/dependabot-automerge-label.yml@9752b04e2e27d6cbc6a06908345e85db2b576abf # v1 | ||
| with: | ||
| # fetch-metadata's package-ecosystem slug for npm/pnpm, not | ||
| # dependabot.yml's "npm" value — see the callee's own comment on | ||
| # this naming split. | ||
| ecosystems: npm_and_yarn |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| version = 1 | ||
|
|
||
| [merge] | ||
| automerge_label = "automerge" | ||
| method = "squash" | ||
| delete_branch_on_merge = true | ||
| blocking_labels = ["wip", "do-not-merge"] | ||
|
|
||
| [approve] | ||
| # Dedicated label, not the general-purpose `automerge` merge-queue label: | ||
| # auto_approve_labels grants Kodiak's own bot approval to anything carrying | ||
| # this label regardless of author. Reusing `automerge` would let any | ||
| # contributor with label-write access get an unreviewed PR auto-approved | ||
| # just by attaching it. | ||
| # | ||
| # dependabot-automerge is only *intended* to be applied by | ||
| # dependabot-automerge-label.yml, after its own semver/ecosystem check | ||
| # confirms the PR is a provable patch/minor dependency bump — never by | ||
| # hand. GitHub doesn't restrict which repo collaborators can attach an | ||
| # *existing* label, and Kodiak's evaluation here only checks label | ||
| # presence, not who added it or the PR's author, so anyone with | ||
| # Triage/Write access could still self-apply it to their own PR for an | ||
| # unreviewed auto-approval. Accepted risk, matching derivita/infrastructure | ||
| # #1716: this repo's collaborators are trusted, and required status checks | ||
| # still gate every PR regardless of label. | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This mitigation does not hold on this repo today. Two consequences of merging as-is:
The description flags branch protection as unverified — this is the verification, and none of it is in place. Worth landing a ruleset on Two things that did check out, for what it's worth:
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Confirmed, current state matches: This needs a ruleset/branch-protection change on |
||
| auto_approve_labels = ["dependabot-automerge"] | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GitHub reports both owner lines in this file as invalid.
gh api repos/derivita/protobuf-javascript/codeowners/errors?ref=nathan/kodiak-dependabot-automergereturnsUnknown owneron lines 5 and 10:@derivita/engineeringexists (13 members), but no org team has access granted to this repo, and CODEOWNERS requires the team be publicly visible with write access to resolve.Effect: these two rules are inert. Even once "Require review from Code Owners" is enabled,
/.github/and/.kodiak.tomlcarry no owner requirement — so a new workflow file (which can run on its own PR with a write-scopedGITHUB_TOKEN) or an edit that weakensblocking_labelsmerges with no code-owner review, which is precisely what this file exists to prevent. It fails silently: GitHub surfaces the error in the CODEOWNERS editor but doesn't block anything.The fix is a repo-settings change rather than a diff change — grant
@derivita/engineeringwrite access to this repo, then confirm thecodeowners/errorsendpoint comes back empty. Flagging it as blocking because the description raises this only as "is this the right team?", where the actual state is "the entry does nothing."There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Confirmed, current state matches:
gh api repos/derivita/protobuf-javascript/codeowners/errors?ref=nathan/kodiak-dependabot-automergestill returnsUnknown owneron lines 5 and 10, andgh api repos/derivita/protobuf-javascript/teamsreturns an empty list — no team currently has any access to this repo, so there's no better team to redirect these entries to.This is a repo-settings/access grant (give
@derivita/engineeringwrite access to the repo), not a file change, so I'm leaving it for a human with the appropriate admin access rather than attempting it myself. Not resolving this thread.