feat: support credentials vending in directory namespace

[jackye1995]

This PR introduces the credentials vending feature to the namespace impl, allowing us to vend credentials if we run directory namespace, or run it as backend for rest namespace. This would allow us to fully test the credentials vending code path end to end.

The actual vending logic mainly consults the same feature implemented in Apache Polaris. The support covers aws, gcp and azure.

[github-actions]

Code Review: Credential Vending for Directory Namespace

Summary

This PR adds credential vending support for AWS, GCP, and Azure cloud storage in the directory namespace. The implementation follows patterns similar to Apache Polaris credential vending.

Feedback

P1: GCP Credential Vending - Expiration Time Calculation

In gcp.rs, the expiration time is calculated by adding duration_millis to the current time:

let expires_at_millis = std::time::SystemTime::now()
    .duration_since(std::time::UNIX_EPOCH)
    .expect("time went backwards")
    .as_millis() as u64
    + self.config.duration_millis;

However, GCP access tokens typically have a fixed 1-hour lifetime regardless of what the caller requests. This can mislead clients into thinking credentials are valid longer than they actually are. Consider:

1. Using the actual expiration from the token if available, or
2. Capping expires_at_millis to 1 hour, or
3. Adding documentation about this limitation

P1: GCP Permission Level Has No Effect

The GCP vendor accepts a permission config but the comment acknowledges it has no actual effect:

Note: GCP OAuth2 tokens dont support fine-grained permissions, so the permission is for API consistency.

This could be confusing for users. Consider either:

1. Not accepting permission for GCP and documenting this clearly, or
2. Adding a log warning when non-default permissions are requested for GCP

P1: S3 Policy Prefix Pattern May Be Incorrect

In aws.rs, the ListBucket condition uses:

if prefix.is_empty() {
    "*".to_string()
} else {
    format!("{}/*", prefix_trimmed)
}

This pattern prefix/* wont match the prefix itself - only objects underneath it. For a table at path/to/table, files like path/to/table/_versions/ need to be listed, but this condition should probably be path/to/table* to also allow listing at the exact prefix level.

P0: Azure - Unused tenant_id Configuration

The AzureCredentialVendorConfig accepts tenant_id but it is never used when creating credentials. DefaultAzureCredential is used unconditionally:

let credential = DefaultAzureCredential::create(azure_identity::TokenCredentialOptions::default())

Either remove the unused config option or implement tenant-specific credential selection.

Testing

The code has unit tests for URI parsing and policy building. Integration tests exist but are marked #[ignore] requiring real cloud credentials. Consider adding integration tests to CI with test credentials if feasible.

Minor Notes

• The documentation is comprehensive
• The builder pattern API is well-designed
• Feature flags are properly configured to make dependencies optional

[github-actions]

PR Review: Credentials Vending for Directory Namespace

Summary

This PR adds credential vending support for cloud storage (AWS S3, GCP GCS, Azure Blob) in the directory namespace. The implementation follows a similar pattern to Apache Polaris credential vending.

---

P0: Critical Issues

1. Missing enabled property requirement in Python/Java docs

The Python and Java documentation shows credential vendor properties but doesn't mention the credential_vendor.enabled=true property which is required to enable credential vending (as checked by has_credential_vendor_config()). Users following the documentation examples won't get credential vending enabled.

In python/python/lance/namespace.py:

>>> # With AWS credential vending (requires credential-vendor-aws feature)
>>> ns = lance.namespace.DirectoryNamespace(
...     root="s3://my-bucket/data",
...     credential_vendor="aws",  # This property doesn't exist
...     credential_vendor_aws_role_arn="arn:aws:iam::123456789012:role/MyRole",
...     credential_duration_millis="3600000"
... )

Should be:

>>> ns = lance.namespace.DirectoryNamespace(
...     root="s3://my-bucket/data",
...     credential_vendor_enabled="true",  # Required
...     credential_vendor_aws_role_arn="arn:aws:iam::123456789012:role/MyRole",
... )

---

P1: Important Issues

2. Incomplete comment in VendedPermission enum

In credentials.rs:77-84, there are incomplete doc comments:

/// For example,
/// It is arguable that if a user has write permission,
///
/// Here these permission sets are more for

This looks like unfinished documentation that should be completed or removed.

3. GCP credential vending doesn't scope to location

The GCP implementation in vend_credentials parses the URI but doesn't use bucket or prefix to scope the credentials:

async fn vend_credentials(&self, table_location: &str) -> Result<VendedCredentials> {
    let (_bucket, _prefix) = Self::parse_gcs_uri(table_location)?;  // Unused!
    let (access_token, expires_at_millis) = self.get_token().await?;
    ...
}

Unlike AWS (which uses scoped IAM policies) and Azure (which scopes to container), GCP tokens are not scoped to the specific bucket/prefix. This should be documented as a limitation, or implement Credential Access Boundaries for proper scoping.

4. Missing integration tests

The PR has unit tests for URI parsing and policy generation, but lacks integration tests for actual credential vending. Given this is security-critical functionality, integration tests that verify the vended credentials work correctly (even mocked) would provide important coverage.

---

Minor Observations (not blocking)

• The AWS policy correctly uses the prefix condition for s3:ListBucket to scope listing
• Azure SAS token generation properly limits user delegation key to 7 days (Azure maximum)
• Test coverage exists for permission levels (Read, Write, Admin) in AWS policy generation

---

Recommendation: Address P0 (documentation) and P1 items #2 (incomplete comment) before merging. Item #3 (GCP scoping) can be addressed as a follow-up with proper documentation of the limitation.

🤖 Generated with Claude Code

[github-actions]

Code Review: Credentials Vending Feature

Summary

This PR introduces credential vending functionality for AWS (STS AssumeRole), GCP (OAuth2 tokens), and Azure (SAS tokens) to the directory namespace implementation. The design follows Apache Polaris patterns and uses feature flags for optional dependencies.

---

P0 - Critical Issues

1. GCP credential expiration is synthetic, not actual
rust/lance-namespace-impls/src/credentials/gcp.rs:127-131

The GCP implementation calculates expires_at_millis by adding config.duration_millis to the current time, but GCP OAuth2 tokens have their own fixed expiration (typically 1 hour) that may differ from the configured duration. This can lead to credential failures if the configured duration exceeds the actual token lifetime.

// Current code adds config duration blindly
let expires_at_millis = std::time::SystemTime::now()
    .duration_since(std::time::UNIX_EPOCH)
    .expect("time went backwards")
    .as_millis() as u64
    + self.config.duration_millis;

Recommendation: The google-cloud-auth crate's token response contains an expires_in field. Use the actual expiration from the token response, or at minimum use min(config.duration_millis, actual_expiration).

---

P1 - Important Issues

2. Unused configuration fields in GCP vendor
rust/lance-namespace-impls/src/credentials/gcp.rs:106-111

The service_account and credentials_file config fields are never used in the get_token() implementation. This is misleading to users who configure these properties expecting them to work.

// These fields are configured but never read
pub service_account: Option<String>,
pub credentials_file: Option<String>,

Recommendation: Either implement service account impersonation / custom credentials file support, or remove these unused fields and document that only default credentials are supported.

3. Azure SAS token expiry calculation relies on requested duration, not key validity
rust/lance-namespace-impls/src/credentials/azure.rs:193-208

The code calculates expires_at_millis using end_time (based on config.duration_millis), but the actual user delegation key validity is capped at key_end_time (max 7 days). If end_time > key_end_time, the returned expiration will be incorrect since the SAS token is bound by the key's validity period, not the requested duration.

// key_end_time may be less than end_time
let key_end_time = if end_time > max_key_end {
    max_key_end  // The actual key constraint
} else {
    end_time
};
// ... but expires_at_millis uses end_time unconditionally
let expires_at_millis = (end_time.unix_timestamp() * 1000 + ...) as u64;

Recommendation: Use key_end_time (or the actual SAS token expiry) for expires_at_millis, not the requested end_time.

4. AWS IAM policy missing s3:HeadObject permission
rust/lance-namespace-impls/src/credentials/aws.rs:150-159

The read policy includes s3:GetObject and s3:GetObjectVersion but omits s3:HeadObject. Many S3 clients (including object_store) use HEAD requests to check object existence or get metadata, which will fail with the current policy.

Recommendation: Add s3:HeadObject to the read action list.

---

Minor Suggestions (non-blocking)

• Test coverage: The integration tests are marked #[ignore] and require manual setup. Consider adding mock-based unit tests for the core policy generation and parsing logic.

• Documentation consistency: The Python docstring uses underscores in property names (credential_vendor_aws_role_arn) while the Rust implementation uses dots (credential_vendor.aws_role_arn). Ensure the Python bindings correctly translate these.

---

Overall this is a well-structured implementation. Addressing the expiration calculation issues (P0/P1) will prevent credential failures in production use cases.

[github-actions]

PR Review: Credentials Vending Feature

This PR introduces credential vending functionality for directory namespace, allowing temporary cloud credentials to be generated for table access. The implementation follows Apache Polaris's approach.

Summary

The PR adds:

• AWS STS AssumeRole with scoped IAM policies
• GCP OAuth2 tokens
• Azure SAS tokens with user delegation keys
• Auto-selection of vendor based on URI scheme (s3://, gs://, az://)
• Integration with DirectoryNamespace via describe_table()

---

P0/P1 Issues

1. [P1] GCP vendor does not implement permission-scoped access (gcp.rs:125-140)

Unlike AWS (which generates scoped IAM policies) and Azure (which sets SAS permissions), the GCP implementation just returns an OAuth2 token without any scope restriction. The permission field is stored but never used to constrain access:

// GCP OAuth2 tokens don't support fine-grained permissions,
// so the permission is for API consistency.

Consider either:

• Implementing Credential Access Boundaries (CAB) to scope the token
• Documenting this limitation prominently (not just in code comments)
• Or returning an error if users expect permission scoping on GCP

2. [P1] Missing validation for duration bounds (aws.rs, azure.rs)

AWS STS AssumeRole has duration limits (900-43200 seconds), but duration_millis is passed directly without validation:

let duration_secs = self.config.duration_millis.div_ceil(1000) as i32;

Values outside the valid range will cause runtime errors from AWS. Consider clamping or validating during config construction.

3. [P1] Incomplete comment in VendedPermission (credentials.rs:76-82)

The VendedPermission enum has incomplete documentation:

/// For example,
/// It is arguable that if a user has write permission,
///
/// Here these permission sets are more for

These appear to be unfinished thoughts that should be completed or removed.

---

Minor Observations (Not Blocking)

• The AWS integration tests use a hardcoded bucket name (jack-lancedb-devland-us-east-1) - consider making this configurable via environment variable
• The Azure SDK has a typo in the method name (get_user_deligation_key) which the code correctly accommodates with a comment

---

Testing

The PR includes good unit test coverage for:

• URI parsing for all three providers
• IAM policy generation for different permission levels
• Config builder patterns

Integration tests are properly marked as #[ignore] and require real cloud credentials.

---

Overall this is a solid implementation of credential vending. The P1 issues should be addressed before merging.

[codecov]

Codecov Report

❌ Patch coverage is 49.88435% with 650 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
rust/lance-namespace-impls/src/credentials/aws.rs	37.27%	307 Missing and 1 partial ⚠️
rust/lance-namespace-impls/src/credentials/gcp.rs	49.69%	162 Missing and 1 partial ⚠️
rust/lance-namespace-impls/src/credentials.rs	73.61%	76 Missing ⚠️
...ust/lance-namespace-impls/src/credentials/azure.rs	51.28%	76 Missing ⚠️
rust/lance-namespace-impls/src/dir.rs	28.94%	26 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

[github-actions]

Code Review for PR #5566: Credentials Vending in Directory Namespace

Thank you for this comprehensive addition of credential vending support. The implementation follows the Apache Polaris approach well. Here are a few concerns to address:

P0: Security Concern - Credentials Logging

aws.rs:267, gcp.rs, azure.rs: The VendedCredentials struct implements Debug which will expose secrets (access keys, tokens) in logs/debug output. Consider:

• Implement a custom Debug that redacts sensitive fields
• Or remove the derive and implement manually

impl std::fmt::Debug for VendedCredentials {
    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
        f.debug_struct("VendedCredentials")
            .field("storage_options", &"[REDACTED]")
            .field("expires_at_millis", &self.expires_at_millis)
            .finish()
    }
}

P1: Configuration Inconsistency - Python/Java vs Rust Property Names

The Python/Java documentation uses underscores (credential_vendor_aws_role_arn) while the Rust implementation uses dots (credential_vendor.aws_role_arn). This will cause user confusion:

• Python (namespace.py:99): credential_vendor_aws_role_arn
• Java (DirectoryNamespace.java:67): credential_vendor_aws_role_arn
• Rust (dir.rs:143): credential_vendor.aws_role_arn

Please align the property naming convention across all bindings.

P1: Missing Test Coverage for Error Cases

The unit tests cover success paths well, but could benefit from:

• Test for invalid URI formats passed to vend_credentials
• Test for malformed properties (e.g., invalid duration values)

Minor Suggestions (Non-blocking)

1. azure.rs:183: The comment // note: typo in the library method name for get_user_deligation_key is helpful. Consider filing an issue upstream if not already done.

2. GCP roles documentation (gcp.rs:12-18): Good explanation of why legacy roles are used. This is helpful context for future maintainers.

3. Integration tests: Well-structured with proper cleanup and unique path generation to avoid test conflicts.

---

Overall, this is a solid implementation. The P0 security issue should be addressed before merging.

[github-actions]

Code Review

This PR introduces credential vending for directory namespace, supporting AWS, GCP, and Azure. The implementation follows Apache Polaris patterns. Here are the key findings:

P0 Issues (Must Fix)

1. Secret key logging risk (rust/lance-namespace-impls/src/credentials/aws.rs)

   • The AwsCredentialVendor struct derives Debug, which includes the sts_client. While the STS client itself doesn't contain secrets, the vended credentials stored in VendedCredentials will be logged if printed with debug formatting. Consider implementing a custom Debug that redacts sensitive fields in VendedCredentials, or add a warning in documentation.

2. GCP token expiration is synthetic (rust/lance-namespace-impls/src/credentials/gcp.rs:125-130)

   • The expiration time is calculated as now + config.duration_millis, but GCP access tokens have their own actual expiration (typically 1 hour). This could lead to users believing credentials are valid when they've actually expired. The code should use the actual token expiration from the get_token() response if available.

P1 Issues (Should Fix)

3. Incomplete doc comment (rust/lance-namespace-impls/src/credentials.rs:82-84)

   /// For example,
   /// It is arguable that if a user has write permission,
   ///
   /// Here these permission sets are more for
   

   These doc comments are incomplete/truncated. Either complete them or remove the fragments.

4. Unused config fields in GCP vendor (rust/lance-namespace-impls/src/credentials/gcp.rs)

   • service_account and credentials_file config fields are defined but never used in get_token(). Either implement service account impersonation or document that these fields are for future use.

5. Python doc mismatch (python/python/lance/namespace.py)

   • Python docs show property format with underscores (credential_vendor_aws_role_arn) but the Rust code uses dots (credential_vendor.aws_role_arn). The Python bindings should clarify which format is expected.

Suggestions (Non-blocking)

• The Azure SDK has a typo in its API (get_user_deligation_key). Consider adding a comment noting this is intentional to match the SDK's typo.
• Good test coverage with unit tests and integration tests (marked as #[ignore]). The integration tests require environment setup which is appropriate.
• The builder pattern for configs is well-designed and follows Rust idioms.

Overall, this is a solid implementation. The main concerns are around the GCP token expiration accuracy and the incomplete documentation.

[github-actions]

PR Review: Credentials Vending in Directory Namespace

Summary

This PR introduces credential vending for AWS (STS AssumeRole), GCP (OAuth2 with CAB), and Azure (SAS tokens) in the directory namespace implementation, similar to Apache Polaris.

P0 Issues

1. Thread Safety Issue: std::env::set_var in GCP vendor (gcp.rs:172)

if let Some(ref creds_file) = config.credentials_file {
    std::env::set_var("GOOGLE_APPLICATION_CREDENTIALS", creds_file);
}

Setting environment variables at runtime is not thread-safe in Rust and can cause undefined behavior in multi-threaded contexts. This is especially dangerous in a server context where multiple credential vendors may be created concurrently.

Recommendation: Pass the credentials file path directly to the google-cloud-auth library if it supports it, or document that this configuration option should only be set before any concurrent operations begin. Alternatively, load credentials from the file directly rather than relying on the environment variable.

---

P1 Issues

2. AWS ListBucket Condition May Be Overly Permissive (aws.rs:163-174)
The ListBucket condition uses StringLike with s3:prefix condition:

"Condition": {
    "StringLike": {
        "s3:prefix": if prefix.is_empty() {
            "*".to_string()
        } else {
            format!("{}/*", prefix_trimmed)
        }
    }
}

When prefix is empty, this allows listing the entire bucket. For multi-tenant scenarios, ensure this is the intended behavior.

3. Missing Test Coverage for Core Functionality
The credential vending code paths (vend_credentials for each provider) are only exercised by integration tests marked with #[ignore]. There are no mock-based unit tests for:

• Error handling when STS/SAS/OAuth calls fail
• Edge cases in URI parsing
• Policy generation for edge-case paths

---

Minor Observations (Non-blocking)

• Good: Secrets are properly redacted in VendedCredentials::Debug implementation
• Good: Duration values are properly clamped to valid ranges for AWS STS
• Good: Builder pattern provides clean configuration API
• Good: Feature flags isolate cloud-specific dependencies

Questions for Maintainers

1. Is credential vending intended for multi-tenant use cases? If so, the empty-prefix ListBucket permission should be restricted.
2. Should the GCP credentials file path be validated/checked for existence before being set as an environment variable?

[github-actions]

Code Review: feat: support credentials vending in directory namespace

Summary

This PR introduces credential vending for AWS, GCP, and Azure cloud storage providers in the DirectoryNamespace. The implementation follows patterns similar to Apache Polaris and provides temporary, scoped credentials for table access.

---

P0 Issues (Must Fix)

1. Missing serde dependency in feature-gated code

The GCP module uses serde::{Deserialize, Serialize} but the serde dependency is only listed as optional in Cargo.toml without being tied to the credential-vendor-gcp feature:

# Current:
google-cloud-auth = { version = "0.18", optional = true }

# Missing:
serde = { workspace = true, optional = true }  # needs to be enabled by credential-vendor-gcp

This will cause a compile error when building with credential-vendor-gcp feature but without another feature that enables serde.

2. GCP reqwest dependency may conflict with existing usage

The PR adds reqwest as an optional dependency for GCP credential vending, but doesn't specify feature flags. This could cause issues if the existing crate uses reqwest with different features elsewhere.

---

P1 Issues (Should Fix)

1. Documentation inconsistency: gcp_credentials_file property mentioned but not implemented

In lib.rs doc comments (line ~93):

credential_vendor.gcp_credentials_file = "/path/to/creds.json"

However, this property is not implemented in gcp_props module or handled in create_gcp_vendor(). The GCP module correctly uses ADC, but the docs suggest a different mechanism. Either remove this from docs or implement it.

2. Potential credential caching concern

The CredentialVendor is created once at namespace initialization and stored as Arc<dyn CredentialVendor>. Each call to vend_credentials() generates new credentials (e.g., new STS AssumeRole call for AWS). For high-frequency describe_table calls, this could result in:

• Rate limiting from cloud providers
• Unnecessary latency

Consider adding credential caching with TTL-based refresh (similar to how AWS SDK handles credentials internally).

3. Missing test coverage for Azure implementation

AWS has integration tests (marked #[ignore]), and GCP has unit tests for the access boundary building. Azure only has a basic config test. Consider adding:

• Unit tests for SAS token permission building
• Integration tests similar to AWS (even if ignored)

---

Suggestions (Non-blocking)

1. Consider adding metrics/tracing: For production debugging, it would be helpful to emit tracing events when credentials are vended.

2. Error message improvement in Azure: When azure_account_name is missing, the error could mention that it's required for credential vending specifically.

---

Overall, this is a well-structured implementation with good separation between cloud providers. The feature gating approach is clean and the API design follows the builder pattern used elsewhere in the codebase.

[github-actions]

Code Review: PR #5566 - Credentials Vending for Directory Namespace

Summary

This PR adds credential vending support to the directory namespace implementation, enabling temporary scoped credential generation for AWS S3, GCP Cloud Storage, and Azure Blob Storage.

---

P0 Issues (Must Fix)

1. Missing Test Coverage for Critical Paths

The credential vending logic lacks integration tests. While there are unit tests for policy building and parsing, there are no tests for the actual vend_credentials methods or the integration with DirectoryNamespace.describe_table(). Given this is a security-sensitive feature, integration tests (possibly using mocks for STS/GCS/Azure endpoints) would significantly reduce risk.

Recommended: Add at least mock-based integration tests for credential vending flows.

---

P1 Issues (Should Fix)

2. Potential Silent Failures in Provider Detection (credentials.rs:274-290)

The create_credential_vendor_for_location function returns Ok(None) when the required provider configuration is missing (e.g., AWS without aws_role_arn). This is mixed with the case where the provider is not detected. Users may not understand why credentials are not being vended.

#[cfg(feature = "credential-vendor-aws")]
"aws" => create_aws_vendor(properties).await,  // Returns Ok(None) if role_arn missing

Recommended: Consider logging a warning when enabled=true but required configuration is missing.

3. AWS Policy Prefix Condition May Be Overly Permissive (aws.rs:140-150)

The S3 ListBucket condition uses a prefix that allows listing with a trailing wildcard:

"s3:prefix": format!("{}/*", prefix_trimmed)

This works but the * at the end is unnecessary for prefix matching and could be confusing since S3 prefix conditions use string matching, not glob patterns.

Recommended: Change to format!("{}*", prefix_trimmed) (the slash is already included when prefix is non-empty).

4. GCP Access Boundary CEL Expression Path Matching (gcp.rs:266-272)

The CEL expression for prefix restriction may not correctly handle paths without trailing slashes:

resource.name.startsWith('projects/_/buckets/{}/objects/{}')

If prefix_trimmed = "data", this allows access to data-other/ as well. Consider appending a / delimiter for non-root prefixes.

5. Optional Feature Dependencies Not Documented in Cargo.toml

reqwest and serde are listed as optional for GCP but are likely already available as workspace dependencies. The feature gate setup appears correct, but confirm these don't inadvertently enable features on reqwest.

---

Minor Observations (Informational)

• Good use of redact_credential to avoid logging sensitive values
• Builder pattern for vendor configs follows project conventions
• The typo in Azure SDK method name (get_user_deligation_key) is noted in the code - this is upstream
• Documentation is comprehensive

---

Security Considerations

• Least Privilege: The permission scoping (Read/Write/Admin) is correctly implemented across all three providers
• Duration Clamping: AWS correctly clamps to 900-43200 seconds, Azure limits to 7 days
• No Secrets Logged: Debug impl for VendedCredentials properly redacts storage options

Overall the security model is sound. The main concern is ensuring misconfigurations are visible to users.

[github-actions]

PR Review: feat: support credentials vending in directory namespace

Summary

This PR adds credential vending support for AWS, GCP, and Azure to the directory namespace implementation. The feature allows vending temporary, scoped credentials based on table location URI schemes.

Review

Overall: Well-structured implementation with good documentation and test coverage. A few items to address:

---

P1 Issues

1. Missing VendedPermission re-export (rust/lance-namespace-impls/src/lib.rs)

   VendedPermission is used in public API (e.g., AwsCredentialVendorConfig::with_permission) but is not re-exported from lib.rs. Users would need to import from the credentials module directly.

   pub use credentials::{
       ..., VendedPermission, ...
   };

2. Potential secret exposure in storage_options (rust/lance-namespace-impls/src/credentials/aws.rs:286)

   The aws_secret_access_key is included in storage_options which is returned via DescribeTableResponse. While this is necessary for the client to use the credentials, consider:

   • Documenting this security consideration
   • Ensuring the response is only sent over encrypted channels
   • Consider adding a warning log when vending credentials over non-TLS connections (if detectable)

3. Missing reqwest dependency for GCP (rust/lance-namespace-impls/Cargo.toml)

   The credential-vendor-gcp feature lists dep:reqwest but reqwest is not declared as a dependency in the [dependencies] section. This would cause a compilation error when the feature is enabled.

---

P0 Issues

None identified.

---

Suggestions (non-blocking)

• The integration tests are well-designed but marked with #[ignore]. Consider adding a CI workflow that runs these with appropriate AWS credentials in a test environment.

• The VendedCredentials::is_expired() check uses SystemTime::now() which could panic if system time goes backwards. Consider using checked_duration_since or handling the error gracefully.

• The GCP availability_condition CEL expression is complex. Consider adding more test cases for edge cases (e.g., prefixes that are substrings of other prefixes).

[Xuanwo]

can we use reqsign instead of all SDKs from aws, azure and gcp?

[jackye1995]

let me try

[jackye1995]

looks like reqsign currently cannot perform scoped credentials generation, so cannot really use it at this moment

[Xuanwo]

Got it, will take a look

[github-actions]

Code Review: Credentials Vending Feature

P0 - Must Fix

1. Azure API Typo - Potential Future Breakage
rust/lance-namespace-impls/src/credentials/azure.rs:154

The code calls get_user_deligation_key() (note: "deligation" instead of "delegation"). This appears to be a typo in the upstream azure_storage_blobs library. While it works now, this is fragile and could break when the library fixes the typo. Please verify this is the correct method name and add a comment if it is indeed a library quirk.

2. Credential Information in Logs
Multiple locations (aws.rs, azure.rs, gcp.rs) log partial credential identifiers at INFO level. Even redacted credentials like access_key_id=AKIAIOSF***MPLE reveal information (e.g., distinguishes long-term keys from session tokens). Consider moving credential vending logs to DEBUG level.

P1 - Should Fix

3. Azure Container-Level Scoping Limitation
rust/lance-namespace-impls/src/credentials/azure.rs:218-222

Azure SAS tokens are scoped to containers, not paths. This means requesting credentials for az://container/sensitive/data grants access to the entire container, violating least-privilege. This limitation should be prominently documented (not just in code comments) and users should be warned to use one container per namespace for proper isolation.

4. Missing Input Validation

• Role ARNs are not validated before STS calls
• Duration values are silently clamped instead of returning errors for invalid input

Consider adding basic validation and returning errors for invalid configurations rather than silently accepting them.

5. No Credential Caching
Each vend_credentials() call makes fresh API requests. This could:

• Exhaust cloud provider API quotas under load
• Impact performance for high-frequency operations

Azure delegation keys (valid up to 7 days) are particularly expensive to regenerate on each call.

Test Coverage Gaps

• No integration tests for Azure or GCP (only AWS has ignored integration tests)
• No tests for credential expiration/refresh logic
• No tests verifying actual scoped access (e.g., that Read permission cannot write)
• No malicious input tests (invalid ARNs, extremely long paths)

Positive Observations

• Good credential redaction in redact_credential()
• Proper scoped IAM policies for AWS
• Clean async handling throughout
• VendedCredentials::Debug properly hides sensitive fields
• Consistent use of Result<> with contextual errors

Verdict

The implementation is solid overall with good security practices. Recommended to address P0 issues before merge, with P1 items as follow-up work.