obelix74 opened a new pull request, #3327:
URL: https://github.com/apache/polaris/pull/3327

   Fixes #3325
   
   <!--
   ๐Ÿ“ Describe what changes you're proposing, especially breaking or user-facing 
changes. 
   ๐Ÿ“– See https://github.com/apache/polaris/blob/main/CONTRIBUTING.md for more.
   -->
   
   ## Summary
   Adds support for AWS STS Session Tags when vending S3 credentials, enabling 
correlation between Polaris catalog operations and S3 access in AWS CloudTrail. 
This feature is controlled by a new feature flag 
`INCLUDE_SESSION_TAGS_IN_SUBSCOPED_CREDENTIAL` (default: `false`).
      
   ## Motivation
   When Polaris vends temporary AWS credentials via STS `AssumeRole`, there's 
no way to correlate S3 access events in CloudTrail back to the originating 
Polaris catalog operation. This makes audit trails incomplete and security 
investigations difficult.
   
   AWS STS Session Tags solve this by attaching metadata to the assumed role 
session that appears in CloudTrail events for all subsequent API calls made 
with those credentials.
      
   ## Changes
   ### New Components
      - **`CredentialVendingContext`** - Immutable value class holding session 
tag context (catalog name, namespace, table name, request-id)
      - **`INCLUDE_SESSION_TAGS_IN_SUBSCOPED_CREDENTIAL`** - Feature flag in 
`FeatureConfiguration` (default: `false`)
   
   ### AWS Integration
      - **`AwsCredentialsStorageIntegration`** - Adds 5 session tags to 
`AssumeRoleRequest` when feature enabled:
        - `polaris:principal` - The authenticated principal name
        - `polaris:catalog` - The catalog being accessed
        - `polaris:namespace` - The namespace being accessed
        - `polaris:table` - The table being accessed
        - `polaris:request-id` - The originating request ID for correlation
      - Tags are marked as **transitive** for role chaining support
      - Tag values truncated to 256 characters (AWS limit)
      - Missing values use `"unknown"` placeholder for consistent CloudTrail 
presence
   
   ### Cache Key Updates
      - **`StorageCredentialCacheKey`** - Now includes 
`CredentialVendingContext` when session tags enabled
      - **`StorageCredentialCache`** - Includes principal in cache key when 
session tags OR principal-name-in-session features are
      enabled (prevents credential cross-contamination)
      - Added `toSanitizedLogString()` for safe debug logging without exposing 
sensitive context
      
   ### Interface Updates
      - Updated `PolarisStorageIntegration`, `PolarisCredentialVendor`, 
`StorageCredentialsVendor` to accept
      `CredentialVendingContext`
      - Updated Azure and GCP integrations for signature compatibility 
(pass-through only)
      
   ## Testing
      - **6 new unit tests** in `AwsCredentialsStorageIntegrationTest`:
        - `testSessionTagsIncludedWhenFeatureEnabled`
        - `testSessionTagsNotIncludedWhenFeatureDisabled`
        - `testSessionTagsWithPartialContext`
        - `testSessionTagsWithLongValues`
        - `testSessionTagsWithEmptyContext`
        - `testSessionTagsAccessDeniedGracefulHandling`
      - All existing tests updated for signature compatibility
      - Full test suites pass: `polaris-core:test`, 
`polaris-runtime-service:test`
      
   ## Configuration
      To enable session tags, set in your Polaris configuration:
      polaris:
        features:
          INCLUDE_SESSION_TAGS_IN_SUBSCOPED_CREDENTIAL: "true"
      
   ## IAM Requirements
      When enabled, the IAM role's trust policy must allow `sts:TagSession`:
      {
        "Effect": "Allow",
        "Principal": { "AWS": "arn:aws:iam::ACCOUNT:root" },
        "Action": ["sts:AssumeRole", "sts:TagSession"]
      }
      
   ## Documentation
      - Code review findings documented in 
`docs/aws-sts-session-tags-review-findings.md`
      
   ## Breaking Changes
      None. Feature is off by default and all interface changes are backward 
compatible.
   
   
   ## Checklist
   - [x] ๐Ÿ›ก๏ธ Don't disclose security issues! (contact [email protected])
   - [x] ๐Ÿ”— Clearly explained why the changes are needed, or linked related 
issues: Fixes #3325
   - [x] ๐Ÿงช Added/updated tests with good coverage, or manually tested (and 
explained how)
   - [x] ๐Ÿ’ก Added comments for complex logic
   - [ ] ๐Ÿงพ Updated `CHANGELOG.md` (if needed)
   - [ ] ๐Ÿ“š Updated documentation in `site/content/in-dev/unreleased` (if needed)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to