Normalize string role claims before claim-based role mapping evaluation

[Yanhaoxi]

Summary

Fixes #6305. Claim-based role mappings are evaluated with the fixed CEL expression claim_value in claims[role_claim_key], whose semantics depend on the role claim value's runtime type. cel-go v0.30.0 defines in only for list/map operands, so a string-typed role claim raised a "no such overload" error that SelectRole swallowed and treated as a non-match — the user silently got FallbackRoleArn even when their claim exactly equaled the configured Claim. An object-typed role claim made in test map-key membership, spuriously matching when the configured value was a key.

WHAT changed:

• Add normalizeRoleClaim, applied to the role claim value before evaluation: string → one-element list (exact membership restored); missing claim → empty list (fallback preserved); any other shape (object, number, bool, null) → rejected.
• SelectRole now fails closed on unsupported claim shapes and claim-mapping evaluation errors (ErrNoRoleMapping, HTTP 403 via the aws_sts middleware) instead of silently granting FallbackRoleArn; matcher-expression errors keep their skip-and-fall-back behavior but log at Warn instead of Debug.
• Add regression tests for string claims (exact match, substring non-match) and unsupported shapes (fail closed).

Type of change

• Bug fix
• New feature
• Refactoring (no behavior change)
• Dependency update
• Documentation
• Other (describe):

Test plan

• Unit tests (task test)
• E2E tests (task test-e2e)
• Linting (task lint-fix)
• Manual testing (describe below)

Run: go test ./pkg/auth/awssts/... ./pkg/vmcp/auth/strategies/.... New tests TestRoleMapper_SelectRole_StringRoleClaim and TestRoleMapper_SelectRole_UnsupportedRoleClaimShape FAIL without the source fix and PASS with it; full suites pass. go vet and gofmt clean; golangci-lint reports only 3 pre-existing gci issues in files this PR does not touch.

API Compatibility

• This PR does not break the v1beta1 API. Configuration shape is unchanged.

Changes

File	Change
pkg/auth/awssts/role_mapper.go	Normalize role claim to a list before evaluation; fail closed on unsupported shapes and claim-mapping evaluation errors; matcher eval-error log Debug → Warn
pkg/auth/awssts/role_mapper_test.go	Add string-claim and unsupported-shape regression tests

Does this introduce a user-facing change?

Yes. Two intentional behavior changes:

1. A string-typed role claim exactly equal to the configured Claim now selects the mapped role (e.g. groups: "admins" with Claim: "admins" → AdminRole); previously it silently fell back. Strings merely containing the value still do not match.
2. A role claim value that is neither a string nor a list (object, number, bool, null) now fails closed (HTTP 403) instead of silently getting the fallback role — and instead of being spuriously matched as a map key for objects. Deployments must emit the role claim as a string or a list of strings.

Special notes for reviewers

• A missing role claim still falls back (empty list), preserving the pre-existing behavior that relied on the swallowed CEL error.
• Matcher-based expressions keep their skip-and-fall-back semantics; only the log level changes (Debug → Warn).
• The fail-closed behavior for unsupported shapes is deliberate: it closes both the silent fallback (string) and the map-key spurious match (object) for the same expression.

[codecov]

Codecov Report

❌ Patch coverage is 85.71429% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.98%. Comparing base (0302206) to head (86ecc66).
⚠️ Report is 14 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/auth/awssts/role_mapper.go	85.71%	7 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6306      +/-   ##
==========================================
+ Coverage   72.85%   72.98%   +0.13%     
==========================================
  Files         742      742              
  Lines       77804    78437     +633     
==========================================
+ Hits        56683    57248     +565     
- Misses      17145    17198      +53     
- Partials     3976     3991      +15     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[reyortiz3]

Thanks for the clear writeup and tests — this closes a real silent-fallback bug. One correctness issue found, plus two minor nits.

pkg/auth/awssts/role_mapper.go: the fail-closed early return discards already-found higher-priority matches

SelectRole collects matches into matches across the whole loop, then sorts by priority and returns matches[0] only after the loop finishes. But when a claim-based mapping's evalContext/EvaluateBool call errors (unsupported shape, or an eval failure), SelectRole returns "", err immediately — discarding any matches already appended from earlier iterations.

Concretely: a config with a matcher-based mapping (e.g. claims.sub == 'admin1', priority 1) and a claim-based mapping sharing the same RoleClaim (e.g. groups). If a request's groups claim has an unsupported shape (an object), the matcher mapping may legitimately match first and get appended to matches, but the claim-based mapping's normalization error later in the same loop throws that result away — the caller gets HTTP 403 even though a legitimate, higher-priority rule already granted access.

Suggest evaluating all mappings first (collecting per-mapping errors alongside matches), and only failing closed if no valid match exists among what was found — rather than aborting the whole loop on the first error encountered.

Minor: normalizeRoleClaim re-clones the full claims map per mapping

For a config with N claim-based mappings sharing the same RoleClaim, evalContext calls normalizeRoleClaim once per mapping, each doing a full O(len(claims)) clone with an identical result. Could be hoisted to compute the normalized claims map once before the mapping loop.

Minor: the clone logic is duplicated inside normalizeRoleClaim

The "allocate clone sized len(claims)+1, copy every key/value" block appears twice (missing-claim branch and string branch). A small helper would avoid the two copies drifting if either needs to change later.

[Yanhaoxi]

Thanks for the review — addressed all three points.

• Claim-based role-claim normalization is now computed once and shared across claim-based mappings; matcher mappings continue to evaluate against the original claims.
• Mapping evaluation no longer returns early on a claim-based normalization or evaluation error. Valid matches are collected and selected by priority; the request fails closed only when no valid mapping matches.
• Extracted the duplicated claims-cloning logic into a small helper.
• Added a regression test showing that a valid, higher-priority matcher mapping still wins when a later claim-based mapping sees an unsupported role-claim shape.

The PR has been updated.