🤖 fix: fail closed on a missing CoderMCPServer access group

[ThomasK33]

Summary

When a group in spec.access.groups does not exist in Coder, the dormant CoderMCPServer reconciler now fails closed. It still removes the access of every group that the list does not name, adds no group, and disables the server if the same generation disables it. The kind is not registered, so users see no change.

Fixes #313

Background

Before this PR, a missing group (GroupNotFound) stopped the sync before it changed anything (fail static, plan 4.8 item 3). A broader group of the earlier generation, such as Everyone, then kept read access to the enabled config until the new group existed. That also held when the same generation disabled the server. The maintainer decided Option B (fail closed) on #313, to match the fail-closed rule of #330 and #360.

How it works

1. syncMCPServerACLPass (the ACL helper of 🤖 feat: add the CoderMCPServer ACL helper #294): when a name does not resolve, it still reads the ACL. It keeps only the group entries whose names access.groups lists, matched by the group names in Coder's ACL answer. Everyone is matched by its ID, which is the organization ID. It removes every other group entry and every user entry, adds nothing, and returns GroupNotFound.
2. syncMCPServer: when that remove-only pass before the config PATCH returns GroupNotFound, and the sparse PATCH of this generation sets enabled: false, the reconciler sends one PATCH that holds only enabled: false (disableForMissingGroup). It checks the answer's ID, organization, slug and enabled. A failed PATCH waits 30 s.
3. The generation stays unapplied, and GroupNotFound resyncs every 10 minutes. Once the group exists, the next sync applies the rest of the generation and adds the missing groups.

A new config with a missing group stays disabled with the nonce, as before. Its default Everyone entry now goes too.

The access.groups field doc describes the behavior, and the dormant CRD is regenerated. The plan's M9 how-to row repeats it.

Validation

• TestMCPServerACLGroupNotFound: for loose matches only, one missing name, lowercase everyone, and a rejected search, the sync gives GroupNotFound with one ACL read and one remove-only ACL write. The default Everyone entry and the user entry go, and no group is added.
• TestMCPServerACLMissingGroupNeverWidens: the ACL holds Everyone, data and analytics. A sync for data, data-science and missing keeps data, removes Everyone and analytics, and does not add data-science although it resolves. The next sync writes nothing. Once missing exists, the sync adds data-science and missing.
• TestCoderMCPServerApplyMissingGroupFailsClosed: an applied config readable by Everyone and data gets a generation with data, ops (missing) and a new description, with and without enabled: false. The reconcile gives GroupNotFound, the ACL becomes data, and the description waits. With enabled: false, the writes are the ACL removal and then {"enabled":false}. Once ops exists, the generation applies.
• TestCoderMCPServerApplyGroupNotFound: a new config with a missing group still gets no config PATCH. Its Everyone entry goes.
• TestMCPServerACLRejectsNamesOutsideThePattern: a name outside the pattern still sends no search for it.
• Mutation checks: each of these changes makes a test fail: the old fail-static return, keeping Everyone when it is not listed, keeping every entry in the by-name pass, and no disable PATCH. One change survives: dropping the !byName guard on the add loop, because the resolver returns no IDs when a name is missing. I kept the guard, so the rule does not depend on that detail.
• Local gates on this head: make verify-vendor, make build, make lint, GOFLAGS=-mod=vendor go tool govulncheck ./..., make test, make test-integration, go test -race ./internal/controller/, and make docs-check pass. make codegen, make manifests, make build-installer and make docs-reference leave no diff.

Size

163 lines added and 14 removed. 5 changed lines are regenerated (the dormant CRD). The rest are hand-written: 29 changed lines in the ACL helper, 25 in the apply step, 5 in the field doc, and 113 in tests.

Refs #155

---

Generated with xum • Model: anthropic:claude-opus-5-5 • Thinking: high • Cost: $195.76

[ThomasK33]

@codex security review

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-14T14:26:46.803000Z	67c3a1d	PR opened
🔒 Security Review	✅ Completed 2026-10-14T14:26:59.964047Z	67c3a1d	Manual request

ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[chatgpt-codex-connector]

🛡️ Codex Security Review

Security review completed. No security issues were found in this pull request.

Reviewed commit: 67c3a1d3a0

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

• You comment "@codex security review"
• A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.