Skip to content

fix(controlplane): restrict owner-role invitations to org owners (CP-1) - #3263

Merged
migmartri merged 3 commits into
chainloop-dev:mainfrom
migmartri:fix/cp-1-owner-invite-guard
Jul 6, 2026
Merged

migmartri merged 3 commits into
chainloop-dev:mainfrom
migmartri:fix/cp-1-owner-invite-guard

Conversation

@migmartri

@migmartri migmartri commented Jul 5, 2026 •

Copy link
Copy Markdown
Member

Fixes #3264

What

OrgInvitationUseCase.Create accepted any caller-supplied role verbatim. The Casbin authz middleware gates OrgInvitationService/Create with PolicyOrganizationInvitationsCreate (admin/owner only), but RoleAdmin.IsAdmin() short-circuits the middleware — so an Admin could invite a new external email as MEMBERSHIP_ROLE_ORG_OWNER. AcceptPendingInvitations then auto-creates an Owner membership, bypassing PolicyOrganizationManageOwners (the Owner-only manage-owners gate) and bootstrapping a fresh Owner via the invitation path.

Change

  • When the requested invitation role is RoleOwner, the biz layer now enforces PolicyOrganizationManageOwners against the caller's authz subject. The check is shared with the existing owner-scoped guards in MembershipUseCase.UpdateRole / DeleteOther through a single enforceManageOwners helper, with the denial messages defined once. Non-owner callers receive ErrUnauthorized("only organization owners can invite owners").
  • The service layer passes the caller's authz subject (org role for users, api-token:<id> for API tokens) down via a new callerRole parameter; the group/project invitation flows pass the requester's membership role.
  • This intentionally also applies to instance admins: they hold PolicyOrganizationInvitationsCreate but deliberately not PolicyOrganizationManageOwners, so they can no longer seed owners into organizations they administer.
  • GroupUseCase.handleNonExistingUser now guards against a nil requester membership instead of dereferencing it.

Parts of this change were produced with AI assistance (Claude Code); see the commit trailers.

🤖 Posted by Maximus bot (Claude Code) on behalf of @migmartri

Review in cubic

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 8 files

Re-trigger cubic

@migmartri

Copy link
Copy Markdown
Member Author

Re: the behavior-change note on instance-admin / owner distinction — in practice Admin and Owner currently behave the same in Chainloop, so this distinction has historically been moot. However, we have plans to enable explicit Owner-only features downstream (e.g. billing, org deletion, SSO management), so we're tightening the boundary now so the PolicyOrganizationManageOwners gate is consistently enforced across both MembershipUseCase.UpdateRole/DeleteOther and OrgInvitationUseCase.Create. The new behavior (only Owners can invite Owners) is the intended forward-looking invariant.

🤖 Posted by Maximus bot (Claude Code) on behalf of @migmartri

@migmartri
migmartri requested a review from a team July 5, 2026 17:27
@migmartri
migmartri force-pushed the fix/cp-1-owner-invite-guard branch from 9988cd0 to c08f461 Compare July 5, 2026 17:28
@chainloop-platform

chainloop-platform Bot commented Jul 5, 2026 •

Copy link
Copy Markdown
Contributor

AI Session Analysis

Avg score Sessions Failing policies Attribution Files Lines Total Duration
🟢 87% 1 ⚠️ 1 100% AI / 0% Human 4 +218 / -238 2h54m31s

🟢 87% — 100% AI — ⚠️ 1 policies failing

Jul 5, 2026 17:27 UTC · 2h54m31s · $118.22 · 766.9k in / 385.4k out · claude-code 2.1.201 (claude-fable-5)

View session details ↗

Change Summary

  • Shares the manage-owners guard between membership and org-invitation flows.
  • Adds a nil-membership guard in group.go.
  • Reworks org invitation integration tests into table-driven role and API-token cases.

AI Session Overall Score

🟢 87% — Verified, aligned change with one gap: no human confirmation after the autonomous finish.

AI Session Analysis Breakdown

🟢 94% · scope-discipline

🟢 The commit staged only four relevant files and left unrelated WIP out. · High Impact

🟢 90% · alignment

No notes.

🟢 90% · solution-quality

No notes.

🟢 88% · context-and-planning

🟢 AI restated findings 3–7 before edits, and /simplify added structured cleanup guidance. · Medium Impact

🟢 88% · user-trust-signal

No notes.

🟡 78% · verification

🟢 The owner-guard tests were expanded and rerun successfully. · High Impact

🟠 After the autonomous finish, no user turn confirmed the behavior worked; verification relied entirely on automated checks. · Medium Severity

💡 When the session turns autonomous, capture one concrete manual or reviewer-facing validation note before declaring completion.


File Attribution

████████████████████ 100% AI / 0% Human

Status Attribution File Lines
modified ai app/controlplane/pkg/biz/orginvitation_integration_test.go +195 / -214
modified ai app/controlplane/pkg/biz/membership.go +15 / -6
modified ai app/controlplane/pkg/biz/orginvitation.go +1 / -17
modified ai app/controlplane/pkg/biz/group.go +7 / -1

Policies (4, 1 failing)

Status Policy Material Messages
✅ Passed ai-config-ai-agents-allowed ai-coding-session-321ef4 -
✅ Passed ai-config-no-dangerous-commands ai-coding-session-321ef4 -
⚠️ Failed ai-config-no-secrets ai-coding-session-321ef4
  • Potential secret (Quoted API key/password) found in session content [turn=114, source=queue-operation, line=25, value=token:' ...ns ']
  • Potential secret (Quoted API key/password) found in session content [turn=116, source=user-message, line=25, value=token:' ...ns ']
  • Potential secret (Quoted API key/password) found in session content [turn=132, source=tool_result, line=22, value=token:")...b, "]
  • Potential secret (Quoted API key/password) found in session content [turn=132, source=tool_result, line=23, value=token:")...hat']
✅ Passed ai-config-mcp-servers-allowed ai-coding-session-321ef4 -

Powered by Chainloop and Chainloop Trace

@migmartri
migmartri force-pushed the fix/cp-1-owner-invite-guard branch from f5650b9 to e034f25 Compare July 6, 2026 10:33
@migmartri
migmartri requested review from a team and matiasinsaurralde July 6, 2026 10:34

@matiasinsaurralde matiasinsaurralde left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

migmartri added 2 commits July 6, 2026 16:43
OrgInvitationUseCase.Create accepted any caller-supplied role verbatim, so an

Admin (who passes the Casbin middleware via IsAdmin() short-circuit) could

invite a new external email as MEMBERSHIP_ROLE_ORG_OWNER. AcceptPendingInvitations

then auto-creates an Owner membership, bypassing PolicyOrganizationManageOwners

(the Owner-only manage-owners gate) and bootstrapping a fresh Owner via the

invitation path.

Add an enforceManageOwners check in the biz layer: when the requested role is

RoleOwner, enforce PolicyOrganizationManageOwners against the caller's role.

This mirrors the existing guard in MembershipUseCase.UpdateRole/DeleteOther.

The service layer passes the caller's authz subject down via a new callerRole

parameter; the two existing biz callers (group/project invitation flows) pass

the requester's membership role. Non-owner callers now receive

ErrUnauthorized("only organization owners can invite owners").

Assisted-by: Claude Code
Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>
- share a single enforceManageOwners helper between MembershipUseCase and
  OrgInvitationUseCase, with the denial messages defined once
- guard against a nil requester membership in GroupUseCase.handleNonExistingUser
- rework TestCreateOwnerGuard as a table-driven test, drop unused setup and
  pin the intentional instance-admin denial
- replace s.T().Run subtests with suite-style s.Run in the invitation tests

Assisted-by: Claude Code
Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>

Chainloop-Trace-Sessions: 321ef432-c960-4e92-bc01-9e70c8dba97d
Assisted-by: Claude Code
Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>
@migmartri
migmartri force-pushed the fix/cp-1-owner-invite-guard branch from b116b18 to c56d555 Compare July 6, 2026 14:54
@migmartri
migmartri requested review from a team and matiasinsaurralde July 6, 2026 15:04
@migmartri
migmartri enabled auto-merge (squash) July 6, 2026 15:09
@migmartri
migmartri merged commit 3cbb22d into chainloop-dev:main Jul 6, 2026
14 checks passed
@migmartri
migmartri deleted the fix/cp-1-owner-invite-guard branch July 6, 2026 16:56
social4hyq pushed a commit to social4hyq/homebrew-core that referenced this pull request Sep 20, 2026
chainloop-cli 1.104.0

Created-by: HarmonybrewBot
Commit-by: HarmonybrewBot
Merged-by: HarmonybrewBot
Description: Created by `brew bump`

---

Created with `brew bump-formula-pr`.<details>
  <summary>release notes</summary>
  <pre>## What's Changed
* Bump Helm Chart and Dagger Version => v1.103.1 by @github-actions[bot] in chainloop-dev/chainloop#3259
* fix(ci): align secrets-scan material name with contract by @migmartri in chainloop-dev/chainloop#3262
* fix: harden GitHub Actions workflows against zizmor findings by @migmartri in chainloop-dev/chainloop#3260
* feat: add opencode chainloop-trace plugin by @migmartri in chainloop-dev/chainloop#3267
* ci: bump docker/login-action to v4.4.0 by @matiasinsaurralde in chainloop-dev/chainloop#3261
* fix(controlplane): restrict owner-role invitations to org owners (CP-1) by @migmartri in chainloop-dev/chainloop#3263
* feat: introduce PR mode for attestations by @migmartri in chainloop-dev/chainloop#3268
* docs: fix broken GUAC integration link in README by @tknura in chainloop-dev/chainloop#3272
* fix(cli): correct registered integration add usage string to --opt by @tknura in chainloop-dev/chainloop#3271
* fix: upgrade extras/dagger deps to resolve 4 security advisories by @migmartri in chainloop-dev/chainloop#3277
* fix(crafter): surface auto-discovery validation errors by @waveywaves in chainloop-dev/chainloop#2983
* docs: regenerate CLI reference for registered integration add usage by @matiasinsaurralde in chainloop-dev/chainloop#3279
* fix: upgrade Go toolchain to 1.26.5 for GO-2026-4970-stdlib by @chainloop-platform[bot] in chainloop-dev/chainloop#3278
* feat(cli): explode archive materials in chainloop att add by @javirln in chainloop-dev/chainloop#3254
* fix(materials): do not record container image digest as tag by @tknura in chainloop-dev/chainloop#3281
* fix(cli): accept matching --name and metadata.name in contract commands by @tknura in chainloop-dev/chainloop#3282
* fix(dispatcher): improve fanout delivery timeouts and retry budget by @migmartri in chainloop-dev/chainloop#3270
* refactor: remove the public attribute from workflows by @matiasinsaurralde in chainloop-dev/chainloop#3283
* fix(cli): preserve verbatim contract bytes when splitting apply documents by @javirln in chainloop-dev/chainloop#3233


**Full Changelog**: https://github.com/chainloop-dev/chainloop/compare/v1.103.1...v1.104.0</pre>
  <p>View the full release notes at <a href="https://github.com/chainloop-dev/chainloop/releases/tag/v1.104.0">https://github.com/chainloop-dev/chainloop/releases/tag/v1.104.0</a>.</p>
</details>
<hr>

See merge request: Harmonybrew/homebrew-core!14158
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Admins can bootstrap organization Owners via owner-role invitations

2 participants