CODEOWNERS: pkgscan-allow.toml entries need the security owner by bryan-minimal · Pull Request #1113 · gominimal/pkgs

bryan-minimal · 2026-10-26T22:41:29Z

Adds packages/*/pkgscan-allow.toml @bryan-minimal to .github/CODEOWNERS.

Why. gominimal/pkgmgr-rs#1105 made packages/<name>/pkgscan-allow.toml the place where a triaged pkgscan finding stops gating the unattended update, which mirrors bytes and opens PRs. An entry is a security decision, so the security owner reviews each one. pkgmgr-rs#1121 adds the matching agent norm: agents may draft an entry but never merge one.

The main ruleset already has require_code_owner_review: true, so this takes effect on merge. CODEOWNERS uses the last matching pattern, so for these files the line replaces * @gominimal/minimalists instead of adding to it.

Decision for the reviewer: possible deadlock. Agent sessions open pkgs PRs as bryan-minimal, and GitHub doesn't let an author approve their own PR. The ruleset has no bypass actors. So with @bryan-minimal as the only owner, nobody can approve an allowlist PR opened from Bryan's account, whether an agent or Bryan opened it. Options:

A @gominimal/security team with at least two humans as the owner (my suggestion).
@bryan-minimal @gominimal/minimalists: any one of them approves. That is the same gate as today, plus a review request to Bryan.
Keep the line as is, and open allowlist PRs from a different account.

The line is left exactly as asked, so this is your call. Please pick one before merging. The agent did not merge this; pkgs PRs are merged by humans.

🤖 Generated with Claude Code

packages/<name>/pkgscan-allow.toml clears findings at the gate that pkgmgr-rs applies before it mirrors and opens an update unattended (gominimal/pkgmgr-rs#1105). An entry is a security decision, so the security owner reviews every one. Agents may draft entries but never merge them. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

coderabbitai · 2026-10-26T22:41:39Z

Warning

Review limit reached

Run on-demand review

_{This review includes 1 billable file and costs up to $0.25.}

Ask an admin to make reviews automatic

Open in CodeRabbit

Reviews can continue after your included limit without a manual trigger. An admin must approve usage-based billing.

Or wait 5 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available. Your 69 included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

Learn how review limits work.

Review configuration:

Run configuration

Source mode: Auto
Repository: gominimal/pkgs
Branch: codeowners-pkgscan-allow
File: .coderabbit.yaml
Commit: 000e446
Layers: Repository YAML
Review profile: CHILL
Plan: Essentials
Run ID: f6a62873-0f0c-46b5-99a9-8f53d73a799d

Commits

Reviewing files that changed from the base of the PR and between 44a31a3 and 000e446.

Files selected for processing: 1

.github/CODEOWNERS

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

norrietaylor

The pattern is right. packages/*/pkgscan-allow.toml matches the path pkgmgr-rs builds (pkg_dir.join("pkgscan-allow.toml"), one level under packages/). It comes after *, so it takes precedence for these files. The CODEOWNERS errors API reports no errors at head, and bryan-minimal has admin on the repo. I'm leaving this as a comment, not an approval, because the owner deadlock described in the PR body is still open, and there is a second gap below.

norrietaylor · 2026-10-30T06:54:25Z

+# (packages/<name>/pkgscan-allow.toml, read by pkgmgr-rs). The security owner
+# approves every entry; agents may draft one but never merge it. Listed after
+# `*`, so it replaces that rule for these files: CODEOWNERS uses the last match.
+packages/*/pkgscan-allow.toml	@bryan-minimal


The deadlock in the description is real at head. The main ruleset (id 14240756) has bypass_actors: [] and require_code_owner_review: true. Agent sessions open pkgs PRs as bryan-minimal, and an author cannot approve their own PR. So every allowlist PR an agent drafts would sit unmergeable, and that is the exact flow pkgmgr-rs#1105 sets up. Pick an owner set before merging. Option 1 (a security team with at least two humans) keeps the security-owner intent. Option 2 (@bryan-minimal @gominimal/minimalists) means any minimalist can approve, the same gate as today, so it would make this rule advisory.

norrietaylor · 2026-10-30T06:54:25Z

@@ -1,2 +1,8 @@
 # Every path requires review from a code owner.
 *	@gominimal/minimalists


.github/CODEOWNERS itself still falls under *. Any one minimalist can approve a PR that deletes or rewrites line 8, and that PR doesn't need the security owner. Add a .github/CODEOWNERS line owned by the same owner set as the pkgscan rule, after *, so nobody can drop the gate without the security owner's approval.

mitodrummer

The pattern is right. packages/*/pkgscan-allow.toml matches both allowlists on main (async-profiler, atuin) and the three in #1263, #1264 and #1265. Every package on main sits at packages/<name>/build.ncl, and pkgmgr-rs only reads <pkg_dir>/pkgscan-allow.toml, so there is no nested path to miss. Because * does not cross /, the pattern matches nothing else. The CODEOWNERS errors API reports no errors at head. Two things block this before merge:

.github/CODEOWNERS:8, the owner set. The main ruleset has no bypass actors, and #1263, #1264 and #1265 are all authored by bryan-minimal. With @bryan-minimal as the only owner, none of them could be approved once this lands. The org has no security team today (only minimalists and webapp-minimalists). So either create one with at least two humans and name it here, or list @bryan-minimal alongside a second named human owner. Don't fall back to @gominimal/minimalists, because that makes the rule advisory.
.github/CODEOWNERS:2. The file itself still falls under *, so any minimalist can approve a PR that removes line 8. Add /.github/CODEOWNERS after the new rule with the same owner set.

norrietaylor

Scope matches the title. The diff adds one CODEOWNERS rule and its comment. Clobber none: git log shows only #713 before this commit, and the diff removes no line. Correctness the pattern is valid. It matches every package directory, because all packages sit one level under packages/. Last-match behavior is as the comment says. No workflow in this repo auto-merges or auto-approves. Two defects remain at this head. Both repeat the open threads r4421165650 and r4421165639, which have no reply and no new commit. They are filed again so that this review does not read as an approval. One owner set fixes both. Not checked: the gh api calls were denied. So the ruleset, the write access of bryan-minimal, the CODEOWNERS errors endpoint and the path that pkgmgr-rs reads are unverified. gitleaks is not installed. I read the six added lines by hand and found no secret.

norrietaylor · 2026-11-04T18:15:37Z

+# (packages/<name>/pkgscan-allow.toml, read by pkgmgr-rs). The security owner
+# approves every entry; agents may draft one but never merge it. Listed after
+# `*`, so it replaces that rule for these files: CODEOWNERS uses the last match.
+packages/*/pkgscan-allow.toml	@bryan-minimal


high · The new gate does not protect itself. .github/CODEOWNERS still falls under *, so one minimalist can approve a PR that deletes line 8 or changes its owner. The next allowlist entry then needs no security owner. Add a .github/CODEOWNERS rule after * with the same owner set as line 8. This repeats thread r4421165650, which has no reply. CODEOWNERS has only these two rules, so the fix is local.

norrietaylor · 2026-11-04T18:15:37Z

+# (packages/<name>/pkgscan-allow.toml, read by pkgmgr-rs). The security owner
+# approves every entry; agents may draft one but never merge it. Listed after
+# `*`, so it replaces that rule for these files: CODEOWNERS uses the last match.
+packages/*/pkgscan-allow.toml	@bryan-minimal


medium · @bryan-minimal is the only owner, and the PR body says agent sessions open pkgs PRs from that account. An author cannot approve their own PR, so an allowlist PR from that account cannot merge. The PR body says the ruleset has no bypass actors. The triaged finding then keeps gating the update until someone opens the PR from another account. Choose the owner set before merge. A team with at least two humans keeps the security intent. This repeats thread r4421165639, which has no reply. I could not read the ruleset myself.

norrietaylor

Follow-up to my Nov 4 review on the same commit: one new high finding inline. The earlier findings still stand.

norrietaylor · 2026-11-05T13:25:20Z

+# (packages/<name>/pkgscan-allow.toml, read by pkgmgr-rs). The security owner
+# approves every entry; agents may draft one but never merge it. Listed after
+# `*`, so it replaces that rule for these files: CODEOWNERS uses the last match.
+packages/*/pkgscan-allow.toml	@bryan-minimal


high · The comment says that the security owner approves every entry and that agents only draft. The rule cannot prove either claim. The PR body says agent sessions act as bryan-minimal, and that is the only account that can give the owner approval. An agent session with that credential can approve an allowlist PR that another account opened. Option 3 in the PR body creates exactly this flow. The entry then clears the unattended supply-chain gate with no human security review. Make the owner a team that excludes every account that agents use. Option 1 fixes this only if bryan-minimal is not the agent account on that team. I did not read the token scope of the agent sessions, so confidence is limited. CODEOWNERS has one such rule, so the fix is local.

agent-137

Review on Karl's behalf. This is a comment, not an approval. Nothing was built for it. The findings come from reading the declarations and fetching upstream sources, and each one says how it was checked. The CI rollup on this head is now SUCCESS.

CI: 11/11 green, and git reports the PR as MERGEABLE. CodeRabbit "SUCCESS" is a false pass: it hit its rate limit, and there is no CodeRabbit review on the head commit. norrietaylor and mitodrummer have left four COMMENTED reviews and five inline threads. None has a reply, and nothing has been pushed since (head 000e446d, 2026-10-26).

Verified clean:

Pattern: packages/*/pkgscan-allow.toml contains a slash, so it is anchored at the repo root, and * does not cross /. It matches all six allowlists on main (async-profiler, atuin, difftastic, opencode, plannotator, rsync) and nothing else.
Precedence: the rule comes after *, so the last match wins for these files, as the comment says.
Format: the tab separator matches line 2.
Conflict with main: none. .github/CODEOWNERS on main has not changed since #713, so the PR is not stale against it.
Owner handle: it resolves (a reviewer reports that the CODEOWNERS errors API is clean and that bryan-minimal has admin). It is a different kind of owner from the rest of the file, which uses only the @gominimal/minimalists team.

Should-fix (these repeat the open threads. I confirmed each from the file and the PR body.)

.github/CODEOWNERS:8: no one can approve allowlist PRs opened from bryan-minimal. @bryan-minimal is the only owner, and authors cannot approve their own PRs. The PR body and a reviewer both report that the ruleset has no bypass actors. Failure scenario: #1384 and every future agent-drafted allowlist PR become unmergeable the moment this lands. Use a team with at least two humans that excludes every account agents use. The PR body's option 2 (adding @gominimal/minimalists) makes the rule advisory.
.github/CODEOWNERS:2: the gate does not protect itself. .github/CODEOWNERS still falls under *. Failure scenario: one minimalist approves a PR that deletes line 8, and the next allowlist entry merges with no security owner. Add a /.github/CODEOWNERS rule after line 8 with the same owner set.
.github/CODEOWNERS:4-5: the comment promises something the rule cannot enforce. It says the "security owner approves every entry; agents may draft one but never merge it". Agent sessions act as bryan-minimal, the only approving owner. So an agent holding that token can approve an allowlist PR that another account opened. That clears the unattended supply-chain gate with no human review. The owner set must exclude agent accounts, or the comment must stop making the claim.

Verdict #1113: the pattern and precedence are correct and there is no conflict with main, but do not merge it as is. The owner set has to change: it deadlocks #1384, the agent account can approve as security owner, and CODEOWNERS itself is unprotected. All three are still open, 13 days after the reviews.

bryan-minimal requested a review from a team as a code owner October 26, 2026 22:41

bryan-minimal enabled auto-merge October 27, 2026 02:31

norrietaylor reviewed Oct 30, 2026

View reviewed changes

This was referenced Nov 2, 2026

difftastic: pkgscan-allow the 0.72.0 sample-file .npmrc findings #1263

Merged

plannotator: pkgscan-allow the 0.28.x CredentialAccess findings (one group is an accepted real capability) #1265

Merged

mitodrummer reviewed Nov 2, 2026

View reviewed changes

norrietaylor reviewed Nov 4, 2026

View reviewed changes

bryan-minimal mentioned this pull request Nov 4, 2026

rsync: allow autoconf's C++11 eval( template in configure.sh (3.5.2, CVE-2026-119741) #1323

Merged

norrietaylor reviewed Nov 5, 2026

View reviewed changes

agent-137 reviewed Nov 9, 2026

View reviewed changes

agent-137 mentioned this pull request Nov 9, 2026

plannotator: allowlist the launchctl test-name match in 0.28.51 (pkgscan false positive) #1384

Merged

bryan-minimal marked this pull request as draft November 11, 2026 19:36

auto-merge was automatically disabled November 11, 2026 19:36
Pull request was converted to draft

Navigation Menu

Repository navigation

CODEOWNERS: pkgscan-allow.toml entries need the security owner - #1113

CODEOWNERS: pkgscan-allow.toml entries need the security owner#1113
bryan-minimal wants to merge 1 commit into
mainfrom
codeowners-pkgscan-allow

bryan-minimal commented Oct 26, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Oct 26, 2026

Review limit reached

Uh oh!

norrietaylor left a comment

Uh oh!

norrietaylor Oct 30, 2026

Uh oh!

norrietaylor Oct 30, 2026

Uh oh!

mitodrummer left a comment

Uh oh!

norrietaylor left a comment

Uh oh!

norrietaylor Nov 4, 2026

Uh oh!

norrietaylor Nov 4, 2026

Uh oh!

norrietaylor left a comment •

edited

Loading

Uh oh!

norrietaylor Nov 5, 2026

Uh oh!

agent-137 left a comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		@@ -1,2 +1,8 @@
		# Every path requires review from a code owner.
		* @gominimal/minimalists

Conversation

bryan-minimal commented Oct 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented Oct 26, 2026

Review limit reached

Uh oh!

norrietaylor left a comment

Choose a reason for hiding this comment

Uh oh!

norrietaylor Oct 30, 2026

Choose a reason for hiding this comment

Uh oh!

norrietaylor Oct 30, 2026

Choose a reason for hiding this comment

Uh oh!

mitodrummer left a comment

Choose a reason for hiding this comment

Uh oh!

norrietaylor left a comment

Choose a reason for hiding this comment

Uh oh!

norrietaylor Nov 4, 2026

Choose a reason for hiding this comment

Uh oh!

norrietaylor Nov 4, 2026

Choose a reason for hiding this comment

Uh oh!

norrietaylor left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

norrietaylor Nov 5, 2026

Choose a reason for hiding this comment

Uh oh!

agent-137 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

bryan-minimal commented Oct 26, 2026 •

edited

Loading

norrietaylor left a comment •

edited

Loading