fix(gcp): resolve binding edge cases and improve error handling

[hughneale]

• Reject organizations/ binding values in resolveCustomRoleTenant() since organization-scope custom role creation via binding field is not yet implemented; direct users to the provider-level organization_id config instead.

• Add explicit detection and warning for partial binding configuration: when some statements have binding set but not all, the code now clearly warns that explicit binding values will be ignored and inference will proceed instead (vs silently falling through).

• Fix Statement.Binding code comment to clarify that organizations/{id} format is NOT supported via the binding field (use provider config instead).

• Fix GCP documentation example that incorrectly stated IAM binding would be applied at folder scope; clarified that when binding resolves to a project, the IAM binding is applied at project scope.

• Update binding resolution order documentation to distinguish between missing binding (all statements) vs partial binding (some statements), each with its own warning behavior.

• Add test cases for organizations/ binding rejection and partial binding fallback behavior (TestResolveCustomRoleTenant now covers 7 cases).

All tests pass. Build succeeds.

Summary

Briefly describe what this PR does and why. Link to the relevant issue or context.

Closes #

---

Type of Change

• feat – New feature (minor version bump)
• fix – Bug fix (patch version bump)
• refactor – Code refactoring, no functional change
• docs – Documentation only
• test – Adding or updating tests
• chore – Build, CI, dependency updates
• major / BREAKING CHANGE – Breaking change (major version bump)

---

What Changed

A concise list of the changes made. Focus on the what and why, not the how.

---

Provider / Workflow / Role Changes

Complete this section if you've added or modified providers, workflows, or roles. Delete if not applicable.

Area	Change	Notes
Provider
Workflow
Role

• Provider config files updated (config/providers/)
• Role config files updated (config/roles/)
• Workflow definitions updated (config/workflows/)
• Example configs updated (examples/)

---

Security Considerations

This project handles privileged access. Describe any security implications of this change.

• No security impact
• Reviewed for least-privilege impact
• Access grant / revocation logic reviewed
• Audit trail is preserved for any new access paths
• No credentials, tokens, or secrets introduced in code or config

Security notes (if applicable):

---

Testing

Describe how this was tested. Include commands if helpful.

• Unit tests pass (go test ./...)
• Functional tests pass
• Integration tests pass
• Manually tested locally — describe scenario below

Manual test scenario (if applicable):

# Describe the steps taken to verify the change works end-to-end

---

Breaking Changes

If this is a breaking change, describe the impact and any migration steps required.

• This PR does not introduce breaking changes
• This PR does introduce breaking changes (describe below)

Migration steps (if applicable):

---

Documentation

• No documentation changes needed
• In-code comments updated
• docs/ updated
• README.md updated
• Config examples updated

---

Checklist

• My branch is up to date with main
• Commit messages follow the conventional commit format (feat:, fix:, major:, etc.)
• No debug code, hardcoded values, or temporary workarounds left in
• All new code has appropriate test coverage
• I have reviewed my own diff before requesting review

[Copilot]

Pull request overview

Updates the GCP provider’s custom-role binding logic to handle folder-tenant edge cases more predictably, with clearer warnings/errors and supporting documentation/tests.

Changes:

• Resolve custom-role creation/binding scope for folder tenants via explicit statement binding (with legacy target-inference fallback).
• Reject unsupported binding values (e.g., organizations/...) and warn on partial binding configuration.
• Add/adjust unit tests and documentation to reflect the new binding behavior and resolution order.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
internal/providers/gcp/rbac.go	Implements binding resolution/inference helpers and applies resolved scope to create/bind/unbind custom roles.
internal/providers/gcp/provider_test.go	Adds unit tests for binding resolution, inference, and rejection cases.
internal/models/role.go	Documents the new Statement.Binding field and provider-specific expectations.
docs/configuration/roles/migration.md	Adds migration guidance for adopting binding and explains warnings.
docs/configuration/roles/index.md	Documents binding, including formats, resolution order, and examples.

Comments suppressed due to low confidence (1)

internal/providers/gcp/rbac.go:1

• The tenantForRole derivation logic is duplicated in both the composite and non-composite branches. Consider extracting a small helper (e.g., tenantForCustomRoleName(roleName, fallbackTenant)) to reduce repetition and keep future edge-case fixes (like org-scoped behavior) consistent across both paths.

package gcp

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 21 out of 21 changed files in this pull request and generated 5 comments.

Comments suppressed due to low confidence (3)

internal/models/provider_rbac.go:556

• The new Statement binding field is dropped during permission validation: when rebuilding validatedStatements, the returned Statement copies ID/Operations/Targets/Conditions but not Binding. This will cause downstream providers (e.g. GCP resolveStatementBindingTenant) to never see the configured binding. Preserve Binding when constructing the validated Statement (and similarly anywhere else statements are rebuilt).

		// Create validated statement with expanded operations
		validatedStatements = append(validatedStatements, Statement{
			ID:         stmt.ID,
			Operations: validatedOperations,
			Targets:    stmt.Targets,
			Conditions: stmt.Conditions,
		})

internal/models/provider_rbac.go:648

• Wildcard expansion currently rebuilds statements without copying the new Binding field, so any configured binding will be lost after expansion. Include Binding: stmt.Binding when appending the expanded Statement.

			if len(ops) > 0 {
				result = append(result, Statement{
					ID:         stmt.ID,
					Operations: ops,
					Targets:    stmt.Targets,
					Conditions: stmt.Conditions,
				})
			}

internal/config/roles.go:1333

• Provider filtering rebuilds statements without preserving the new Binding field. If a role statement sets binding, it will be silently lost after filtering, breaking binding-based scope resolution in providers. Preserve Binding (and consider whether it also needs provider-prefix filtering, analogous to operations/targets).

		// Filter targets by provider
		filteredTargets := c.filterByProvider(stmt.Targets, allowedProviders)

		result = append(result, models.Statement{
			ID:         stmt.ID,
			Operations: filteredOps,
			Targets:    filteredTargets,
			Conditions: stmt.Conditions,
		})

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Comment mismatch: this validator is registered for the new Statement ID field, but the comment says "Statement.Name". Update the comment to reference Statement.ID to avoid confusion (and keep the docs aligned with the struct tags).

Suggested change

	// Used by Statement.Name field — must start with a lowercase letter,
	// Used by Statement.ID field — must start with a lowercase letter,

[Copilot]

This section states "targets continues to control" what resources operations act on, but later in the same document (and in code) GCP targets are explicitly metadata-only and not enforced. Reword this bullet to avoid implying GCP targets enforce scope (e.g. note that target enforcement is provider-specific, and GCP ignores them).

Suggested change

	- `binding` does not restrict which resources the operations act on; `targets` continues to control that.
	- `binding` does not itself restrict which resources operations apply to. Conceptually, `targets` define the resource scope, but enforcement is provider-specific (for example, the GCP provider treats `targets` as metadata only and does not enforce them).

[Copilot]

The doc says statement id is "not shown in notifications or user-facing messages", but the UI template internal/daemon/static/role.html now renders the ID column. Please clarify the doc (e.g. "not shown in Slack/email notifications"), or remove the claim.

Suggested change

	- The value is **not shown** in notifications or user-facing messages; it is an internal identifier only.
	- The value is primarily an internal identifier and is not included in Slack/email notifications, though it may appear in administrative UI tables.

[Copilot]

permissionsToGcpPermissions logs a warning whenever a statement has targets. This function runs during custom role create/patch, which for composite roles can happen frequently; the warning is likely to be noisy (especially since config loading already warns). Consider lowering to Debug, rate-limiting, or emitting once per role/statement (or only when binding is missing and targets are being relied upon).

Suggested change

	logrus.Warnf("GCP custom roles do not enforce statement targets; targets are metadata only. Use binding field to control IAM assignment scope. Targets: %v", stmt.Targets)
	logrus.Debugf("GCP custom roles do not enforce statement targets; targets are metadata only. Use binding field to control IAM assignment scope. Targets: %v", stmt.Targets)

[Copilot]

resolveCustomRoleTenant is added and has dedicated tests, but it doesn't appear to be used anywhere in the provider code path (per-statement resolution uses resolveStatementBindingTenant instead). If it's intentionally unused, consider removing it (and its tests) to avoid dead-code maintenance; otherwise wire it into the authorization/revocation flow where multi-statement roles need a single resolved tenant.