Add support for IAM policies (both managed and inline). by ggreer · Pull Request #132 · ConductorOne/baton-aws

ggreer · 2026-06-30T22:33:04Z

No description provided.

github-actions · 2026-06-30T22:36:15Z

+		})
+	}
+
+	listPoliciesInput := &iam.ListPoliciesInput{}


🟡 Suggestion: ListPoliciesInput{} leaves Scope at its default (All), so every sync also pulls the 1000+ AWS-managed policies. Combined with the principal-ARN reconstruction issue above (AWS-managed policy ARNs use account aws), these produce many broken grants and large syncs. If only customer-managed policies are intended, set Scope: types.PolicyScopeTypeLocal. (confidence: medium)

github-actions · 2026-06-30T22:37:42Z

@-

github-actions

Blocking issues found — see review comments.

stale review

github-actions · 2026-06-30T22:53:14Z

+		})
+	}
+
+	listPoliciesInput := &iam.ListPoliciesInput{}


🟡 Suggestion (medium confidence): ListPolicies with no Scope/OnlyAttached defaults to Scope=All, OnlyAttached=false, which enumerates all ~1000+ AWS-managed policies per account. Each then drives a ListEntitiesForPolicy call in Grants (plus GetUser/GetRole/GetGroup per attached entity), creating many unattached policy resources and a large API/throttling load. Consider OnlyAttached: true (and/or Scope: types.PolicyScopeTypeLocal) to limit to policies actually in use.

github-actions · 2026-06-30T22:53:22Z

+			continue
+		}
+		userARN, err := iamEntityARN(ctx, iamClient, "user", userName)
+		if err != nil {
+			l.Warn("baton-aws: failed to resolve IAM user ARN for managed policy grant, skipping",
+				zap.String("user_name", userName),
+				zap.Error(err),
+			)
+			continue
+		}


🟡 Suggestion (low confidence): entity-ARN resolution failures here are logged and skipped for all error types. A transient/throttling error (e.g. 429 on GetUser) would silently drop that principal's grant and under-report access. Consider propagating retryable errors (let the SDK retry) and only skipping on genuine not-found/permanent failures.

github-actions · 2026-06-30T22:53:27Z

+}
+
+func (o *inlinePolicyResourceType) Grant(_ context.Context, _ *v2.Resource, _ *v2.Entitlement) ([]*v2.Grant, annotations.Annotations, error) {
+	return nil, nil, fmt.Errorf("baton-aws: inline policies cannot be created via provisioning")


🟡 Suggestion: inline_policy advertises CAPABILITY_PROVISION, but Grant returns a plain fmt.Errorf. Returning status.Errorf(codes.Unimplemented, ...) (matching the gRPC-status convention used in Revoke) lets callers distinguish "unsupported" from a real failure.

github-actions · 2026-06-30T23:01:02Z

Connector PR Review: Add support for IAM policies (both managed and inline).

Blocking Issues: 1 | Suggestions: 3 | Threads Resolved: 0
Criteria: Criteria status: loaded .claude/skills/ci-review.md from trusted base 7d7551883228.
Review mode: full
View review run

Review Summary

Full PR diff scanned for security and correctness. This PR adds managed (iam_policy) and inline (inline_policy) IAM policy resource types, emits attached-managed-policy grants from the user/role/group Grants paths, and adds a sync-only-attached-policies config flag. The prior feedback about missing GrantExpandable on inline-policy grants now appears addressed (inline_policy.go Grants calls policyGrantExpansion for role/group parents). No new security issues found; the remaining items below were raised previously and are still open in the current diff.

Security Issues

None found.

Correctness Issues

pkg/connector/inline_policy.go:174-181 (previously raised, still open) — listInlinePolicyNames only handles NoSuchEntityException; on AccessDenied it returns the wrapped error and hard-fails the whole inline-policy List. This is inconsistent with getInlinePolicyDocument (line 236) and the managed-policy paths in iam_user.go/role.go/iam_group.go, which Warn+continue. Existing installs upgraded to this version without the newly required iam:ListRolePolicies/iam:ListGroupPolicies scopes will fail the sync instead of degrading gracefully (R7/B7).

Suggestions

pkg/connector/iam_policy.go:44-45 (previously raised, still open) — ListPolicies leaves Scope unset (defaults to All); with sync-only-attached-policies=false (the default) this enumerates ~1150 AWS-managed policies per account. Because iam_policy is a child of every account, the same AWS-managed ARN (arn:aws:iam::aws:policy/...) is emitted as a duplicate resource under each account parent. Consider Scope=Local for the managed-policy list, or documenting the volume/duplication tradeoff. The new sync-only-attached-policies flag partially mitigates volume.
docs/connector.mdx (previously raised, still open) — not updated for the newly required scopes (iam:ListPolicies, iam:GetPolicy, iam:ListAttachedRolePolicies, iam:ListAttachedGroupPolicies, iam:ListRolePolicies, iam:ListGroupPolicies, and the inline Delete*Policy provisioning scopes). Customers copying it will hit AccessDenied on the new sync/provision paths (D3).
No tests accompany the new code (previously raised, still open) — add coverage for inlinePolicyResourceID/parseInlinePolicyResourceID round-trip, iamRoleNameFromARN, policyGrantExpansion, and the listInlinePolicyNames parent-type switch.

Prompt for AI agents

Verify each finding against the current code and only fix it if needed.

## Correctness Issues

In `pkg/connector/inline_policy.go`:
- Around line 174-181: listInlinePolicyNames only treats NoSuchEntityException as skippable;
  every other error (including AccessDenied) is returned via wrapAWSError, which hard-fails the
  inline-policy List and thus the sync. Mirror the graceful degradation used in
  getInlinePolicyDocument (line 236) and the managed-policy grant paths: when
  isAccessDeniedError(listErr) is true, log a Warn (with parent ARN/type) and return an empty
  result with no error so the inline-policy sync degrades to empty instead of failing when
  iam:ListUserPolicies/iam:ListRolePolicies/iam:ListGroupPolicies is missing.

## Suggestions

In `pkg/connector/iam_policy.go`:
- Around line 44-45: ListPoliciesInput sets OnlyAttached but leaves Scope unset (defaults to
  All), so by default every account enumerates all AWS-managed policies and emits the same
  aws:policy ARNs as duplicate resources under each account parent. Consider setting
  Scope=iamTypes.PolicyScopeTypeLocal, or otherwise deduplicating AWS-managed policies across
  account parents, or document the tradeoff.

In `docs/connector.mdx`:
- Add the newly required IAM permissions to the credential/scope documentation:
  iam:ListPolicies, iam:GetPolicy, iam:ListAttachedRolePolicies, iam:ListAttachedGroupPolicies,
  iam:ListRolePolicies, iam:ListGroupPolicies, and the inline provisioning scopes
  iam:DeleteUserPolicy/iam:DeleteRolePolicy/iam:DeleteGroupPolicy.

In `pkg/connector` tests:
- Add unit tests covering inlinePolicyResourceID/parseInlinePolicyResourceID round-trip
  (including names with special chars), iamRoleNameFromARN for role ARNs with paths,
  policyGrantExpansion for user/role/group principals, and the listInlinePolicyNames
  parent-type switch.

github-actions

No blocking issues found.

github-actions · 2026-06-30T23:22:39Z

+		return nil, nil, err
+	}
+
+	grants, err := listAttachedUserPolicyGrants(ctx, iamClient, userName, resource.Id)


🟡 Suggestion: This propagates an error and hard-fails the whole iam_user Grants phase if ListAttachedUserPolicies returns AccessDenied. role.go handles the equivalent call by logging a Warn and continuing (listAttachedRolePolicyGrants), but here (and in iam_group.go:162 and inline_policy.go List) the error is returned. For existing installs whose IAM policy lacks the newly required iam:ListAttachedUserPolicies / iam:ListGroupPolicies scopes, this turns a missing permission into a full sync failure. Consider degrading gracefully and consistently across all three paths. (confidence: medium)

github-actions · 2026-06-30T23:22:41Z

+	listPoliciesInput := &iam.ListPoliciesInput{
+		// TODO: Only list attached policies?


🟡 Suggestion: ListPolicies with no Scope defaults to Scope=All / OnlyAttached=false, so this enumerates every AWS-managed policy (~1150) per account on every sync, and iam_policy is a child of each account. Since grants only reference policies that are actually attached, consider Scope: types.PolicyScopeTypeLocal and/or OnlyAttached: true to bound the resource count and per-policy traffic (addresses the existing TODO). (confidence: medium)

github-actions

No blocking issues found.

github-actions

No blocking issues found.

github-actions

No blocking issues found.

carolinaroncaglia · 2026-07-01T13:53:10Z

I detected this with my automated connector smoke suite. This change is live on the latest channel in our sandbox, so the smoke picked it up and aws-v2 started failing its sync on 2026-07-01:

sync-grants-for-resource: error listing grants for resource
  group/arn:aws:iam::531807593589:group/test-users failed:
  baton-aws: iam.ListAttachedGroupPolicies failed:
  StatusCode: 403, api error AccessDenied

Two things worth considering:

Policy enumeration isn't gated on the resource type being enabled. In that tenant the IAM Managed Policy and Inline Policy resource types are toggled Off, yet the group-grant sync still calls ListAttachedGroupPolicies (and presumably ListGroupPolicies for inline). If those resource types are disabled, the connector ideally shouldn't enumerate them at all — that avoids requiring the extra IAM permission when the capability isn't in use.
A permission error there aborts the entire sync. The 403 AccessDenied on one group's ListAttachedGroupPolicies failed the whole run — users, roles, SSO, everything — not just the policy data. Since this is a new/optional capability, it may be safer to skip + annotate (warn) on a permission error and let the rest of the sync complete, rather than failing hard.

The 403 itself is because our sandbox role (C1IntegrationTestAWS) lacks iam:ListAttachedGroupPolicies — we can add that, but the gating/resilience above would matter for any customer who enables the connector without the new IAM permissions, or who leaves the policy resource types off.

Happy to share more repro detail.

github-actions · 2026-07-02T20:48:39Z

+	grant := grantSdk.NewGrant(
+		resource,
+		inlinePolicyAttachedEntitlement,
+		resource.ParentResourceId,
+		grantSdk.WithAnnotation(
+			&v2.V1Identifier{
+				Id: V1GrantID(
+					V1MembershipEntitlementID(resource.Id),
+					resource.ParentResourceId.Resource,
+				),
+			},
+		),
+	)


🟡 Suggestion: For group/role parents, this inline-policy grant is emitted without a GrantExpandable annotation, unlike the managed-policy path (grantForAttachedManagedPolicy) which expands group grants to group:...:member (shallow) and role grants to role:...:assignment. AWS inline policies on a group/role apply to members/assumers, so as written those members won't inherit the inline-policy grant. Consider mirroring the managed-policy expansion for group and role principals for consistency. (confidence: medium)

github-actions

Blocking issues found — see review comments.

stale review

ggreer requested a review from a team June 30, 2026 22:33