test(security): probe production admission for foreign team identities

[devantler]

🤖 Generated by the Agentic Engineer

Why

The rule that stops a team object from being pointed at a different GitHub team is deployed, but nobody has yet seen production refuse such a change or let the legitimate ones through. The security issue stays open on that missing proof, and the available operator access cannot obtain it.

What

Adds a manually started check that asks production whether it would refuse a foreign team identity and accept the legitimate changes, using trial requests that are never stored. It runs only from the main branch after a typed confirmation, waits its turn behind deployments, and reports enforced, not enforced, or inconclusive.

Part of #3144

[devantler]

🤖 Generated by the Agentic Engineer

@coderabbitai full review

[coderabbitai]

⚠️ Action not completed

Pull request base or head changed.

[devantler]

🤖 Generated by the Agentic Engineer

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository YAML (base), Organization UI (inherited)
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: 02ba413a-3159-4618-92c9-9b37518d0c34

📥 Commits

Reviewing files that changed from the base of the PR and between 50407ac and 926eb43.

📒 Files selected for processing (4)

• .github/workflows/ci.yaml
• .github/workflows/probe-github-team-external-identity.yaml
• scripts/probe-github-team-external-identity.sh
• scripts/tests/test-probe-github-team-external-identity.sh

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📜 Recent review details

⏰ Context from checks skipped due to timeout. (1)

• GitHub Check: 🧪 Validate Manifests

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2026-08-10T13:01:12.782Z

Learnt from: devantler
Repo: devantler-tech/platform PR: 3057
File: .github/workflows/ci.yaml:622-659
Timestamp: 2026-08-10T13:01:12.782Z
Learning: Repository shell tests and scripts must remain compatible with macOS Bash 3.2. Do not use Bash 4+ features such as `mapfile`; use portable constructs, such as a `while IFS= read -r` loop, instead.

Applied to files:

• scripts/tests/test-probe-github-team-external-identity.sh
• scripts/probe-github-team-external-identity.sh

🔇 Additional comments (4)

scripts/probe-github-team-external-identity.sh (1)

260-264: The swapped probe can skip valid swaps.

The condition other_identity != identity skips another Team that carries the same identity. That case does not occur in practice, so the skip is harmless. The swapped probe also sends another Team's real identity as a patch value. That value is not printed, so the public log stays clean.

scripts/tests/test-probe-github-team-external-identity.sh (1)

1-425: LGTM!

.github/workflows/probe-github-team-external-identity.yaml (1)

1-127: LGTM!

.github/workflows/ci.yaml (1)

641-646: LGTM!

Also applies to: 1804-1812

---

📝 Walkthrough

Walkthrough

Adds a Bash probe that checks GitHub Team external-identity admission through server-side dry-run requests and reports enforced, not-enforced, or inconclusive results. Adds a manually dispatched production workflow with branch and confirmation guards. Updates CI to run ShellCheck and the probe tests when the Kubernetes path filter matches.

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to 926eb

No actionable issue remains in the production admission probe, its guarded workflow, or its CI validation. The PR is mergeable after normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 926eb

The probe sends non-persisting requests and has explicit execution guards. It follows an existing production-access pattern, but adds another workflow using broad production credentials. Live credential restrictions and deployment protections were not independently verified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The intended admission operations target Team objects in github-config, but the workflow declares a cluster-admin production kubeconfig. Compromise of credential-bearing execution could therefore affect the whole production cluster. This credential model already existed in the base diagnostic workflow; the PR adds a caller, not an evidenced privilege grant or new tenant access path. The cloud token’s effective permissions were not established.

Trust Boundaries and Controls

• observed — The supplied workflow is dispatch-only, rejects non-main refs before checkout, pins checkout to the dispatch SHA, and checks exact confirmation before restoring credentials. Ref and confirmation values enter shell through environment variables. GitHub token permissions are limited to contents read, but do not restrict the Kubernetes credential. Remote production-environment protection settings were not independently inspected.

Resilience and Maintainability Implications

• observed — Unrecognized responses are unattributed and prevent ENFORCED. An adoption create that unexpectedly succeeds is also inconclusive, accounting for an object disappearing during the probe. The script removes its local temporary directory on exit and suppresses raw admission errors from normal output.

Hardening Proposals

• proposed — Consider a dedicated probe identity limited to necessary reads and impersonation of the specific applier and provider accounts, with correspondingly narrow permissions for those accounts. This would reduce credential-compromise exposure; it is not a verified bypass in this PR.
• proposed — Consider recognizing adoption success from a canonical API AlreadyExists reason rather than any error containing “already exists.” The current matcher is broader than the intended storage-level outcome, but no production false verdict was demonstrated.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 28.57% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 21 functions across 2 files. (2 skipped: …	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly summarizes the main change: a security probe of production admission for foreign team identities.
Description check	✅ Passed	The description explains why the production admission probe is needed and how it runs and reports results.

Full details: Docstring Coverage

Explanation

Docstring coverage is 28.57% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 21 functions across 2 files. (2 skipped: 2 unsupported.)

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

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[devantler]

🤖 Generated by the Agentic Engineer

Readiness evaluation at 926eb437f417afb112fda1dec59a306b280186f5

• Review: CodeRabbit finished its review of this head with no actionable comments. An independent reviewer pass on the first commit found no correctness or safety problem and three lesser gaps, all fixed in the second commit: a team whose identity the policy does not list yet now ends inconclusive instead of being reported as a defect, the swapped-identity probe has its own test class, and the step summary is checked for leaks.
• Tested: the 28-case suite passes under Bash 3.2 and 5; mutation controls that drop the dry-run flag, stop requiring the policy name in a refusal, accept an audit-mode policy, skip the approved-identity check, or change the concurrency group each fail it.
• Tried as a user: ran the script against production with the read-only operator identity. Every precondition read passed (policy ready and enforcing, two teams with listed identities, one provider account), and because that identity may not impersonate, all twelve trial requests came back unattributed and the run ended inconclusive with exit 3 — the fail-closed path, with no identity, account name or address in the output. A conclusive answer needs the protected workflow, which is what this change adds.
• After merge: dispatch the workflow from main with its confirmation phrase and record the verdict on Team external-name is only pinned at creation, so a tenant UPDATE can still adopt a foreign GitHub team #3144.