Repository navigation
Conversation
Security updates were already enabled at the repository level but every pull request waited for a human, and there was no dependabot.yml, so ordinary version updates were never raised and a security fix tended to arrive as a large jump. Add dependabot.yml for the workflow actions and the optional Python extras, grouped so routine bumps arrive as one pull request rather than one per dependency. The project declares no runtime dependencies, so these updates change CI and the kafka/fuzz extras only, never the shipped package. Add a workflow that approves a Dependabot pull request and turns on auto-merge for a security fix or an ordinary patch/minor bump. A major version is never merged unattended, including for a security advisory, because a major is where a fix is most likely to change behaviour; those are labelled needs-triage and left open with a comment saying why. Merging uses GitHub's auto-merge, which lands the pull request only after the required checks pass. Auto-merge without required checks merges immediately, so main is now protected with the deterministic CI contexts (the six test matrix jobs, lint, docker) as required checks. Administrators are deliberately not included, so direct pushes still work. Also declare explicit least-privilege permissions in ci.yml and fuzz.yml, which clears the four CodeQL "workflow does not contain permissions" alerts. Every workflow in the repository now states its permissions.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9e2eda4c4e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # part of this mechanism, not an optional extra. | ||
| name: Dependabot auto-merge | ||
|
|
||
| on: pull_request |
There was a problem hiding this comment.
Use a trusted event for Dependabot writes
For Dependabot-authored PRs, GitHub treats workflows triggered by pull_request as fork-originated and restricts GITHUB_TOKEN to read-only, even though this job requests write permissions. Consequently, the gh pr edit, gh pr review, and gh pr merge commands fail under the default security model, so routine updates are never auto-merged and major updates are not labeled; use a suitably actor-guarded pull_request_target workflow that does not execute PR code.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
All reported issues were addressed across 4 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
kafka-python backs the kafka extra and is imported at runtime in net.py, so a bump does reach users -- the claim in dependabot.yml that a dependency update never changes the shipped package was wrong. CI has no live broker, so a green matrix is not evidence the extra still works, which is exactly the evidence auto-merge relies on. Excluding it from the grouped pull request is not enough on its own: the group only controls how updates are batched, not whether they merge, so a separate kafka-python pull request would still have merged unattended. The auto-merge workflow now gates on the dependency name as well as the semver level, and holds the pull request whenever a shipped runtime dependency appears -- including when it arrives inside a group. The gate is exact-name matched, so a different package with a similar name is not caught by accident. Verified against the whole matrix: CI-only patch and minor merge; kafka-python holds at patch, minor and major, alone or grouped; any major holds; kafka-python-ng is not mistaken for it. The comment in dependabot.yml now says which updates reach users instead of claiming none of them do.
|
Thanks both. One finding was right and is fixed; the other I checked against GitHub's documentation and am not taking, with reasoning below. cubic — kafka-python is a shipped runtime dependency (fixed in ab1948b)Correct, and the premise in my own comment was wrong. The suggested
Verified against the full matrix:
The misleading comment claiming a dependency update never changes the shipped package is gone; it now names the one that does. Codex —
|
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name=".github/workflows/dependabot-auto-merge.yml">
<violation number="1" location=".github/workflows/dependabot-auto-merge.yml:64">
P1: When Dependabot reports a security major update, `UPDATE_TYPE` is `security-update:semver-major`, so this condition falls through and auto-merges a non-runtime dependency. Match both update-type prefixes, because the policy explicitly excludes all majors, including security advisories.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| names=",$(printf '%s' "$DEPENDENCY_NAMES" | tr -d '[:space:]')," | ||
| reason="" | ||
|
|
||
| if [ "$UPDATE_TYPE" = "version-update:semver-major" ]; then |
There was a problem hiding this comment.
P1: When Dependabot reports a security major update, UPDATE_TYPE is security-update:semver-major, so this condition falls through and auto-merges a non-runtime dependency. Match both update-type prefixes, because the policy explicitly excludes all majors, including security advisories.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/dependabot-auto-merge.yml, line 64:
<comment>When Dependabot reports a security major update, `UPDATE_TYPE` is `security-update:semver-major`, so this condition falls through and auto-merges a non-runtime dependency. Match both update-type prefixes, because the policy explicitly excludes all majors, including security advisories.</comment>
<file context>
@@ -31,24 +46,60 @@ jobs:
+ names=",$(printf '%s' "$DEPENDENCY_NAMES" | tr -d '[:space:]'),"
+ reason=""
+
+ if [ "$UPDATE_TYPE" = "version-update:semver-major" ]; then
+ reason="this is a major version update, where a fix is most likely to change behaviour"
+ else
</file context>
| if [ "$UPDATE_TYPE" = "version-update:semver-major" ]; then | |
| if [ "$UPDATE_TYPE" = "version-update:semver-major" ] || | |
| [ "$UPDATE_TYPE" = "security-update:semver-major" ]; then |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ab1948bf20
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| gh pr review "$PR" --approve \ | ||
| --body "Automatically approved: ${UPDATE_TYPE} for ${DEPENDENCY_NAMES}. These affect CI only; the shipped package is unchanged." |
There was a problem hiding this comment.
Remove the approval that prevents enabling auto-merge
When the repository's Allow GitHub Actions to create and approve pull requests setting is disabled, gh pr review --approve is rejected even when the job grants pull-requests: write; because this script uses set -e, it exits before gh pr merge --auto runs. The change describes enabling auto-merge but not this separate Actions setting, and branch protection deliberately requires no approval, so remove this unnecessary command or explicitly enable the repository setting documented under default GITHUB_TOKEN permissions.
Useful? React with 👍 / 👎.
Security updates were already enabled at the repository level, but every Dependabot pull request waited for a human, and there was no
dependabot.yml— so ordinary version updates were never raised and a security fix tended to arrive as a large jump from a long-stale version.What now merges without a human
needs-triage, commented, left openA major is where a fix is most likely to change behaviour, so it gets a human even when it carries a CVE.
Why branch protection is part of this
GitHub's auto-merge lands a pull request as soon as it is mergeable. With no required status checks, "auto-merge" means "merge immediately, without waiting for CI" — the opposite of what is wanted.
mainis therefore now protected with the deterministic CI contexts as required checks:test (3.9)…test (3.14),lint,dockerDeliberately not required:
fuzz-pr,triage, and the third-party AI reviewers. Those are either event-specific or outside our control, and a flaky external service should not be able to wedge the merge queue.Also deliberate:
enforce_admins: false), so you can still push directly tomainand merge by hand. This adds a gate for the bot, not for you.mainare disabled.Repository settings changed alongside this:
allow_auto_mergeenabled (it was off, so the workflow could not have worked), anddelete_branch_on_mergeenabled so merged Dependabot branches clean themselves up.Dependabot scope
github-actionsandpip, weekly, grouped so routine bumps arrive as one pull request instead of one per dependency. Worth being explicit: the project declares no runtime dependencies, so these updates touch CI and the optionalkafka/fuzzextras only — they never change the shipped package.CodeQL
ci.ymlandfuzz.ymlnow declare least-privilegepermissions: contents: read, clearing alerts #1–#4 ("workflow does not contain permissions"). Every workflow in the repository now states its permissions explicitly.Alerts #5 and #6 (binding a socket to all interfaces,
net.py:76and:97) are not addressed here — see the PR discussion; I believe those are correct behaviour for a syslog receiver rather than defects, and that is a call worth making deliberately rather than in passing.Summary by cubic
Adds automatic merging for safe Dependabot updates and declares least-privilege workflow permissions. Previously every Dependabot PR waited for a human; now security fixes and patch/minor updates merge after CI passes, while major versions and the runtime-shipped
kafka-pythonare left for review.Dependabot auto-merge
dependabot.ymlfor weekly grouped updates to GitHub Actions and pip;kafka-pythonis excluded from the group because it ships to users through thekafkaextra.kafka-pythongetneeds-triagelabel and a comment, never auto-merged.mainwith required CI checks so auto-merge waits for CI; admins are excluded.allow_auto_mergeanddelete_branch_on_mergeenabled.Workflow permissions
permissions: contents: readinci.ymlandfuzz.yml, clearing CodeQL "workflow does not contain permissions" alerts.Written for commit ab1948b. Summary will update on new commits.