Add AWS OIDC trust structural evaluator#138
Conversation
Add the pure, read-only structural evaluator and fail-closed IAM policy parsing required by SPEC-03. Cover stable mismatch codes with table-driven golden tests. Grounded in baton-aws connector source and AWS IAM API documentation per D-NHI-012. Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| type Condition map[string]map[string]StringValues | ||
|
|
||
| // StringValues handles both string and list forms used by IAM policy fields. | ||
| type StringValues []string | ||
|
|
||
| func (values *StringValues) UnmarshalJSON(data []byte) error { | ||
| parsed, err := unmarshalStringOrArray(data) | ||
| if err != nil { | ||
| return fmt.Errorf("condition value must be string or array: %w", err) | ||
| } | ||
| *values = parsed | ||
| return nil | ||
| } |
There was a problem hiding this comment.
🟡 Suggestion: Adding Condition to the shared Statement struct tightens parsing for the existing sync-path callers (extractTrustPrincipals, extractTrustPrincipalsByKind), not just the new evaluator. Previously the Condition key was ignored entirely, so any condition shape parsed. Now StringValues.UnmarshalJSON rejects anything that isn't a string or array-of-strings — IAM permits literal JSON booleans/numbers as condition values (e.g. "Bool":{"aws:MultiFactorAuthPresent":true}, numeric operators). If such a value appears in a role's trust policy, the whole statement fails to unmarshal, causing extractTrustPrincipals to silently drop that role's assume-role grants and extractTrustPrincipalsByKind to fall back to generic NHI classification. Consider coercing scalar bool/number to string in the unmarshaler so the strictness stays scoped to the evaluator rather than the shared type. (medium confidence)
There was a problem hiding this comment.
Fixed in e9e569b. I moved the strict evaluator and its private IAM policy parser into internal/awstrust, restored the shared pkg/connector Statement shape so existing sync parsing continues to ignore arbitrary Condition JSON, and added should ignore non-string condition values covering the boolean example. Full go test ./... and changed-code golangci-lint pass.
Connector PR Review: Add AWS OIDC trust structural evaluatorBlocking Issues: 0 | Suggestions: 0 | Threads Resolved: 0 Review SummaryThe full PR diff was scanned for security and correctness. The new commits migrate resource attributes (profile, status, created-at) off the deprecated trait options ( Security IssuesNone found. Correctness IssuesNone found. SuggestionsNone. |
Move strict OIDC trust parsing into an internal package so it cannot change the connector sync parser. Add a regression test for valid boolean IAM conditions raised in PR review. Decision: D-NHI-012 Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com>
Move profile, status, and created-at values off deprecated trait fields so the required repository lint check passes while preserving resource behavior. Decision: D-NHI-012 Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com>
|
Required CI lint is fixed in 21d497e (not treated as approval-only): migrated all 19 deprecated trait profile/status/created-at fields to canonical Resource attributes and updated affected assertions. Local |
1 participant
Summary
Implements the read-only AWS OIDC trust evaluator core defined by SPEC-03, per D-NHI-012.
internal/awstrustpackage with the pureEvaluateWebIdentityTrust(expected, observed) []TrustMismatchboundarySource grounding
The implementation is grounded in the existing connector and vendored SDK surfaces:
pkg/connector/role.go:(*roleResourceType).Grantsalready callsGetRoleand consumesAssumeRolePolicyDocumentpkg/connector/helpers.go:TrustPolicy,Statement,Action, andPrincipalare the existing trust-policy parser types extended herevendor/github.com/aws/aws-sdk-go-v2/service/iam/api_op_GetOpenIDConnectProvider.go:GetOpenIDConnectProviderOutputprovides URL and client-ID/audience observationsvendor/github.com/aws/aws-sdk-go-v2/service/iam/types/types.go:Roleprovides role ARN, trust policy, and max-session observationsRelevant primary AWS references: GetRole, Role, GetOpenIDConnectProvider, OIDC provider configuration, Principal, and Condition operators.
Exact scope and non-goals
This PR is the internal structural evaluator only. It does not expose an action, add a C1 API/delivery path, mint or accept OIDC tokens, call STS, mutate IAM, persist health state, generate IaC, or claim end-to-end operational readiness. It is independent of baton-sdk #941 and does not modify or depend on baton-aws #137.
Test evidence
gofmton changed Go files: passgo test ./pkg/connector/... -count=1: passgo test ./... -count=1: passgolangci-lint run --new-from-rev=HEAD: pass, 0 issuesgit diff --check: passmake lint: reports 19 pre-existing SDK deprecation (SA1019) findings in unchanged files; no findings are introduced by this diff