feat(iam): add CEL allow-policy evaluator

[snazy]

Summary

Adds the disabled-by-default IAM allow-policy domain model, a restricted Nessie CEL-Java Conditions profile, principal resolution, role expansion, and pure policy evaluation. The CEL environment exposes only the Version 1 declarations and runtime overloads, with unsupported or failed conditions non-granting.

Type of change

• New feature (feat:)

GCP Compatibility

Models the supported Cloud Storage IAM Conditions subset without introducing a general authentication system or changing the default no-auth emulator behavior. Conditional bindings require Uniform Bucket-Level Access.

Follow-up

Bucket-policy APIs and enforcement are introduced by later commits. ACL authorization, project-policy inheritance, custom roles, full UBLA lifecycle behavior, and CEL profile expansion (including extract and regex) remain out of scope.

Validation

Focused CEL, evaluator, normalizer, principal-resolver, configuration, and UBLA tests cover the new pure evaluation surface.

[snazy]

Full IAM/CEL change set (commit-stack) in #135

[greptile-apps]

Greptile Summary

Adds a disabled-by-default IAM allow-policy evaluation foundation.

• Introduces immutable IAM policy, binding, condition, principal, resource, and role models.
• Adds restricted CEL condition compilation and evaluation with Boolean validation and fail-closed runtime behavior.
• Adds principal resolution and pure allow-policy evaluation.
• Adds REST and gRPC support for bucket IAM configuration and Uniform Bucket-Level Access lifecycle behavior.
• Adds configuration documentation and focused unit and integration coverage.

Confidence Score: 5/5

The PR appears safe to merge, with the earlier CEL validation, policy-version normalization, and gRPC IAM mapping concerns addressed in the current code.

No new actionable failures were introduced since the previous review, and no previous finding remains outstanding. The two earlier threads resolved without replies were manually resolved without explanation. The current gRPC mappings carry IAM configuration across create and response paths and preserve the intended FieldMask semantics; the prior whole-update concern was correctly conceded in the existing Greptile reply.

Important Files Changed

Filename	Overview
src/main/java/io/floci/gcp/services/iam/NessieIamConditionEvaluator.java	Implements a restricted CEL environment, Boolean result validation, bounded program caching, and fail-closed evaluation.
src/main/java/io/floci/gcp/services/iam/IamPolicyEvaluator.java	Evaluates supported roles, members, conditions, and policy resources without transport or persistence dependencies.
src/main/java/io/floci/gcp/services/iam/IamPolicyNormalizer.java	Normalizes stored policy data, including mapping the unspecified protobuf version to version 1.
src/main/java/io/floci/gcp/services/gcs/GcsGrpcMapper.java	Maps bucket IAM configuration across gRPC create, response, wildcard, parent-message, and leaf-mask operations.
src/main/java/io/floci/gcp/services/gcs/GcsService.java	Persists IAM configuration and implements Uniform Bucket-Level Access lock-time normalization and validation.
pom.xml	Adds the CEL runtime and compatible protobuf dependency management.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
    Token[Authorization token] --> Resolver[IamPrincipalResolver]
    Resolver --> Principal[IamPrincipal]
    Resource[GCS bucket or object] --> Hierarchy[IamResourceHierarchy]
    Hierarchy --> Policy[IamPolicy]
    Policy --> Evaluator[IamPolicyEvaluator]
    Principal --> Evaluator
    Evaluator --> Roles[IamRoleCatalog]
    Evaluator --> Conditions[NessieIamConditionEvaluator]
    Conditions --> Decision{Condition matches}
    Roles --> Decision
    Decision --> Result[Allow or deny]

Loading

_{Reviews (8): Last reviewed commit: "feat(iam): add CEL allow-policy evaluato..." | Re-trigger Greptile}

[greptile-apps]

gRPC Drops IAM Configuration

The shared bucket model now stores IAM configuration, and REST creation and responses expose it, but the gRPC bucket response and create/update mappings do not carry this field. A bucket configured through REST therefore appears to have no IAM configuration when read through the supported gRPC Storage API. This violates the repository directive to keep GCS behavior aligned across REST and gRPC; add the matching protobuf mappings and cross-protocol coverage.

Context Used: AGENTS.md (source)

Knowledge Base Used: Data, storage, and query services

[greptile-apps]

Whole Updates Preserve Old Values

When a gRPC update mask selects all of iam_config or iam_config.uniform_bucket_level_access, this code merges the request into the stored configuration instead of replacing the selected message. For example, sending enabled=false leaves a stored true unchanged because mergeUniformBucketLevelAccess only writes true values. An iam_config update also cannot clear omitted public-access prevention data. These valid updates are silently ignored, which differs from the REST replacement behavior and violates the repository requirement to keep GCP behavior aligned across REST and gRPC.

Context Used: AGENTS.md (source)

Knowledge Base Used: Data, storage, and query services

[greptile-apps]

You're right. The original finding was based on the earlier implementation, where enabled=false was not written. In the current head, mergeUniformBucketLevelAccess writes the requested value unconditionally, while parent message masks preserve omitted sibling fields and leaf masks can clear individual values. This matches protobuf FieldMask merge semantics and the cited Storage testbench behavior; the wildcard path separately replaces the full IAM configuration. The REST path is also merge/patch based, so replacement here would reduce cross-protocol consistency. Resolving this finding.

[snazy]

@greptile-apps please resolve your comments for the resolved findings.

[greptile-apps]

I confirmed the resolved findings: non-Boolean CEL conditions, default protobuf policy version handling, and nested IAM update sibling deletion. The gRPC IAM mapping and whole-message IAM update semantics findings remain unresolved and intentionally open. I couldn't post this reply because permission to submit the comment was denied.

[snazy]

@greptile-apps I think the "whole-message IAM update semantics findings" finding is resolved.

The requested replacement behavior is not the default protobuf FieldMask contract. When a message field is the final path component, the supplied message is merged into the existing message, preserving omitted sibling fields. Google’s Storage testbench implements UpdateBucket using FieldMask.MergeMessage in the same way:

• Protobuf FieldMask update semantics
• Google Storage testbench UpdateBucket implementation

The concrete enabled=false example no longer applies to the current head. mergeUniformBucketLevelAccess writes the requested Boolean value unconditionally, so a present UBLA submessage can disable a stored true value:

floci-gcp/src/main/java/io/floci/gcp/services/gcs/GcsGrpcMapper.java

Lines 218 to 224 in 8b3d702

	private static Map<String, java.lang.Object> mergeUniformBucketLevelAccess(
	java.lang.Object current, Bucket.IamConfig.UniformBucketLevelAccess requested) {
	Map<String, java.lang.Object> merged = mutableMap(current);
	// Official clients can select the parent IAM field while sending only this
	// submessage. Its presence makes the plain proto3 false value intentional.
	merged.put("enabled", requested.getEnabled());
	return merged;

The covered behaviors are now:

• iam_config and iam_config.uniform_bucket_level_access merge their supplied message while preserving omitted siblings.
• Leaf masks such as iam_config.public_access_prevention can explicitly clear individual fields.
• A wildcard mask replaces the complete configuration and clears omitted IAM configuration.

These cases are covered here:

floci-gcp/src/test/java/io/floci/gcp/services/gcs/GcsGrpcControllerTest.java

Lines 214 to 270 in 8b3d702

	@Test
	void iamConfigurationMessageMaskMergesIntoStoredMessage() {
	assertMessageMaskDisablesUniformAccessWithoutClearingSiblingFields(
	"grpc-iam-parent-mask", "iam_config");
	}
	@Test
	void uniformAccessMessageMaskMergesIntoStoredMessage() {
	assertMessageMaskDisablesUniformAccessWithoutClearingSiblingFields(
	"grpc-ubla-parent-mask", "iam_config.uniform_bucket_level_access");
	}
	@Test
	void wildcardMaskClearsOmittedIamConfiguration() {
	String bucketName = "grpc-iam-wildcard-mask";
	service.createBucket(bucketName, "test-project", BASE_URL, Map.of(
	"iamConfiguration", Map.of(
	"uniformBucketLevelAccess", Map.of("enabled", true),
	"publicAccessPrevention", "enforced")));
	RecordingObserver<Bucket> updated = new RecordingObserver<>();
	controller.updateBucket(UpdateBucketRequest.newBuilder()
	.setBucket(Bucket.newBuilder()
	.setName(GcsGrpcMapper.bucketName(bucketName)))
	.setUpdateMask(FieldMask.newBuilder().addPaths("*"))
	.build(), updated);
	assertNull(updated.error);
	assertFalse(updated.single().hasIamConfig());
	assertEquals(Map.of(), service.getBucket(bucketName).getIamConfiguration());
	}
	private void assertMessageMaskDisablesUniformAccessWithoutClearingSiblingFields(
	String bucketName, String mask) {
	service.createBucket(bucketName, "test-project", BASE_URL, Map.of(
	"iamConfiguration", Map.of(
	"uniformBucketLevelAccess", Map.of("enabled", true),
	"publicAccessPrevention", "enforced")));
	RecordingObserver<Bucket> updated = new RecordingObserver<>();
	controller.updateBucket(UpdateBucketRequest.newBuilder()
	.setBucket(Bucket.newBuilder()
	.setName(GcsGrpcMapper.bucketName(bucketName))
	.setIamConfig(Bucket.IamConfig.newBuilder()
	.setUniformBucketLevelAccess(
	Bucket.IamConfig.UniformBucketLevelAccess.newBuilder()
	.setEnabled(false))))
	.setUpdateMask(FieldMask.newBuilder().addPaths(mask))
	.build(), updated);
	assertNull(updated.error);
	assertFalse(updated.single().getIamConfig()
	.getUniformBucketLevelAccess().getEnabled());
	assertFalse(updated.single().getIamConfig()
	.getUniformBucketLevelAccess().hasLockTime());
	assertEquals("enforced", updated.single().getIamConfig()
	.getPublicAccessPrevention());

The REST update path is also patch/merge based, so replacing a selected gRPC message would make the protocols less consistent rather than more consistent. Could you reconsider and resolve this finding?

[greptile-apps]

You're right—the finding is resolved in the current head. The implementation now follows the relevant protobuf FieldMask merge semantics: parent message masks merge supplied fields while preserving omitted siblings, leaf masks can explicitly clear fields, and * replaces the complete configuration. mergeUniformBucketLevelAccess also writes the requested enabled value unconditionally, so false correctly disables a stored true. The added tests cover these cases, and the REST patch behavior remains consistent. Resolving this finding.

[snazy]

@greptile-apps mind resolving your review comments then?

[greptile-apps]

I confirmed the resolved findings: non-Boolean CEL conditions, default protobuf policy version handling, nested IAM update sibling deletion, and whole-message FieldMask semantics. The gRPC IAM mapping finding remains open because bucket create/update/response mappings still omit IAM configuration. I attempted to submit this status, but permission was denied, so I could not resolve or post the review-thread updates from this session.

[snazy]

@greptile-apps I think, the gRPC IAM mapping finding is resolved now. Maybe perform a fresh, full review against the latest, force-pushed head?

[snazy]

Looks like Greptile's happy (despite it's recent comment versus the "top level comment")