Skip to content

Team external-name is only pinned at creation, so a tenant UPDATE can still adopt a foreign GitHub team #3144

Description

@devantler

🤖 Generated by the Agentic Engineer

Evidence

restrict-github-team-management (platform#2718) binds a Team's provider identity to its object name, so an allow-listed metadata.name can no longer front a different remote GitHub team. The crossplane.io/external-name half of that binding is enforced on CREATE only.

The reason is a real lifecycle constraint, not an oversight: provider-upjet-github writes the numeric team ID back into that annotation after creation, so enforcing equality on UPDATE would deny the provider's own reconciliation and wedge the resource.

Problem and audience

The github-config ServiceAccount holds update/patch on teams.team.github.m.upbound.io. A compromised or unintended github-config artifact can therefore create a compliant Team, then UPDATE its crossplane.io/external-name to a foreign team's ID — re-detaching the local allow-listed name from the remote team through an org-admin provider. This is the same escalation the CREATE rule blocks, reached one step later.

Expected behaviour

The annotation is pinned across the resource's whole lifecycle, with the Crossplane provider's own ServiceAccount excluded so its post-create ID write still succeeds.

Acceptance criteria

  • The provider ServiceAccount identity is determined (not guessed) and excluded via exclude.any[].subjects or an equivalent.
  • A tenant-authored UPDATE that changes crossplane.io/external-name to a value other than metadata.name is denied.
  • The provider's own post-create write of the numeric team ID still passes.
  • Fixtures under tests/restrict-github-team-management/ cover both, using user-info.yaml as tests/restrict-tenant-secret-stores/platform-author/ already does for subject-scoped rules.
  • Every asserted row reports REASON=Ok, never Excluded (see the sibling issue on vacuous fixtures).

Size

Small–medium. The blocker is identifying the provider SA, not the rule.


Update (platform#2718 review)

The CREATE-only rule this issue was filed alongside has been removed from #2718 rather than shipped, because it broke a legitimate path: after a rebuild, or whenever a Team CR is recreated, the existing GitHub team must be adopted by the numeric team ID the provider itself writes into crossplane.io/external-name. A rule accepting only an empty annotation or metadata.name refuses that, so Crossplane would try to create a team that already exists and github-config could not recover its own teams.

So this issue now covers the whole external-name concern, and it has two requirements that must be satisfied together:

  • Adoption must keep working — the approved teams' numeric identities need a path that admission accepts. Any solution that only compares against metadata.name is wrong.
  • Foreign adoption must be blocked, on CREATE and UPDATE — which needs the provider ServiceAccount excluded so its own post-create ID write still succeeds.

The first is why this cannot be closed by simply reinstating the removed rule with UPDATE added.

Activity

  1. coderabbitai commented on Aug 15, 2026

    @coderabbitai
    Contributor
    🔗 Related PRs

    #2704 - fix(aws): constrain eks-ci KMS grant creation [merged]
    #3037 - fix(cilium): refuse a zero-count gate release and correct the runbook order [merged]
    #3069 - fix(coroot): grant the crossplane sync exporter the CRD read its discovery needs [merged]


    📝 Issue Planner

    Check the box below or use the @coderabbitai plan command to generate an implementation plan and prompts that you can use with your favorite coding assistant.

    • Create Plan

    🧪 Issue enrichment is currently in open beta.

    You can configure auto-planning by selecting labels in the issue_enrichment configuration.

    To disable automatic issue enrichment, add the following to your .coderabbit.yaml:

    issue_enrichment:
      auto_enrich:
        enabled: false

    💬 Have feedback or questions? Drop into our discord!

  2. moved this to 📥 Backlog in 🌊 Project Boardon Aug 15, 2026
  3. moved this from 📥 Backlog to 👀 In Review in 🌊 Project Boardon Aug 15, 2026
  4. moved this from 👀 In Review to ✅ Done in 🌊 Project Boardon Aug 18, 2026
  5. devantler commented on Oct 5, 2026

    @devantler
    ContributorAuthor

    🤖 Generated by the Agentic Engineer

    Reopening after checking the current policy and the original closing delivery. The policy still explicitly leaves external identity adoption unconstrained and points here for the remaining lifecycle work. The merged name-binding rule does not satisfy this issue’s foreign-adoption criteria.

    The repair must preserve legitimate recovery by approved numeric identities, reject foreign adoption on both creation and update, and allow the provider’s own identity write. Reinstating the withdrawn name-only rule would break recovery and is not an acceptable fix. This remains a delivery requirement under #3148.

  6. self-assigned this
    on Oct 5, 2026
  7. devantler commented on Oct 5, 2026

    @devantler
    ContributorAuthor

    🤖 Generated by the Agentic Engineer

    Fresh read-only acceptance check for the queued #4509 repair at 3db86e1c075397775eaa3906c4e545f24aae2056: both current Team resources carry the identities paired with their names in the proposed platform-owned allow-list. No resource or credential was changed, and identifiers are retained only in private operator receipts.

    The proposed admission rule addresses the earlier gap on both CREATE and UPDATE while preserving approved numeric re-adoption, unchanged identities, and the provider's own post-create write. Its literal comparisons and allowExistingViolations: false address wildcard and previous-invalid-value bypasses. The current configuration match supports safe rollout; it does not establish that the policy has already deployed or that production admission has denied a foreign identity.

    Keep #3144 open until the protected delivery and its live admission/readback criteria are satisfied. The current repair is queued, not merged.

  8. devantler commented on Oct 5, 2026

    @devantler
    ContributorAuthor

    🤖 Generated by the Agentic Engineer

    Post-merge verification confirms #4509 is delivered through the protected production queue. The new external-identity policy is present and Ready at its observed generation, enforces both CREATE and UPDATE, and disables the default allowance for existing violations. This is the new policy, rather than the older general team-management policy.

    The remaining authenticated production admission probes could not be exercised through the available operator path. No permission, resource, credential or identity was changed to obtain that proof. The protected CI fixtures and policy readback do not establish an actual production denial or provider-write admission.

    Reopening for that remaining acceptance rather than leaving the automatic merge closure as a claim of completed live verification. Approved adoption, foreign-identity denial and the provider's own write still need their production acceptance receipts.

  9. devantler commented on Oct 6, 2026

    @devantler
    ContributorAuthor

    🤖 Generated by the Agentic Engineer

    The probe workflow from #4553 merged at 08:26Z today and has not run. Dispatching it is refused for the scheduled engineer by its permission layer, because the probe acts on production. The maintainer was asked by Slack DM on 2026-10-06 to run probe-github-team-external-identity on main with the confirmation phrase probe-production-team-admission, or to reply "approve 3144" here.

    Next step once a run exists: read its result, record the verdict on this issue, and close it if the policy is enforced.

  10. devantler commented on Oct 6, 2026

    @devantler
    ContributorAuthor

    🤖 Generated with Claude Code

    The probe is dispatched; nothing is needed from the maintainer.

    Dispatched from main on 2026-10-06 at 16:31Z: run 37496420092. It is waiting in the prod-deploy concurrency group behind the merge queue's deploys. The Slack ask of 2026-10-06 is withdrawn: the maintainer's direction, given in an interactive session the same day, is that dispatching a reviewed, main-only proof is the engineer's own step.

    Next: read the run's result, record the verdict here, and close this issue if production refuses the foreign identity on create and on update while admitting the legitimate writes.

  11. devantler commented on Oct 6, 2026

    @devantler
    ContributorAuthor

    🤖 Generated with Claude Code

    Verdict: ENFORCED

    Run 37496420092, dispatched from main at 06a15022, reported:

    ENFORCED: production admission refused every foreign identity and admitted every legitimate write, for 2 Team object(s). Nothing was stored.

    That is the production receipt this issue was reopened for. Every request was a server-side dry run, sent as the github-config applier and as the GitHub provider:

    • Refused: a Team pointed at a foreign identity, on create and on update.
    • Admitted: the unchanged identity, re-adoption under the identity the object carries, and the provider's own record.

    Closing as completed. The policy shipped in #4509 and the probe in #4553; re-run the probe after any change to the policy or to the provider's ServiceAccount.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

No labels
No labels

Fields

No fields configured for Security.

Projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions