fix: expose entitlement guard contract metadata - #1027
Merged
Conversation
|
Warning Review limit reached
More reviews will be available in 18 minutes and 24 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (34)
📝 WalkthroughWalkthrough
ChangesEntitlement Guard Contract Pipeline
Sequence Diagram(s)sequenceDiagram
participant Client as HTTP Client
participant Guard as EntitlementGuard
participant Req as EntitlementRequirement
participant Mgr as EntitlementManager
participant Tel as Telemetry
participant Audit as EntitlementAuditSink
Client->>Guard: canActivate(context)
Guard->>Req: getEntitlementRequirements(controller, handler)
Req-->>Guard: EntitlementRequirement[]
loop 각 requirement
Guard->>Guard: createGuardInput(requirement, context)
Guard->>Mgr: check(guardInput)
Mgr-->>Guard: EntitlementCheckResult {status, granted}
alt status: allowed
Guard->>Tel: recordEvent(entitlement.guard.allowed, attrs)
Guard->>Audit: recordEntitlementGuard(event)
Audit-->>Guard: recorded
else status: denied
Guard->>Tel: recordEvent(entitlement.guard.denied, attrs)
Guard->>Audit: recordEntitlementGuard(event)
Guard-->>Client: throw Problem
end
end
Guard-->>Client: true
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
📊 Benchmark Results❌ Some benchmarks failed Gate failures
Updated: 2026-06-21T04:46:58.370Z · Commit: 9a8a657 |
kang-heewon
force-pushed
the
fix/920-entitlement-guard
branch
3 times, most recently
from
June 20, 2026 07:29
ee82263 to
cfbbaf5
Compare
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/entitlements-core/src/libs/decorators/RequireEntitlement.ts`:
- Around line 20-26: The issue is that on line 25 in the
appendEntitlementRequirement call with propertyKey, using target.constructor is
incorrect for static methods because target is already the constructor function
for static methods, making target.constructor === Function which causes metadata
to be recorded on Function instead of the actual class. To fix this, determine
whether the property is a static method or instance method and pass the correct
target: use target directly for static methods (where target is the constructor
function) and target.constructor for instance methods (where target is the
prototype). You can distinguish between static and instance methods by checking
if target is a constructor function versus a prototype object.
In `@packages/entitlements-core/src/libs/EntitlementGuard.ts`:
- Around line 217-230: The recordEntitlementGuard call on the
EntitlementAuditSink is being directly awaited, which means failures in the
audit sink can cause exceptions that override the guard result and turn allowed
requests into errors. Wrap the await call in a try-catch block so that audit
sink failures are caught and logged separately via telemetry without affecting
the original guard result. The guard should still return its decision regardless
of whether the audit sink succeeds or fails, ensuring that audit failures don't
break the entitlement check flow. Apply this isolation pattern to all
recordEntitlementGuard calls in the file including the ones referenced in the
"Also applies to" section.
- Around line 80-90: The telemetry is currently recording `problem.detail ??
problem.message` as the reason code in the span, which can contain sensitive
information like tenant identifiers or arbitrary custom strings, creating
high-cardinality telemetry. In the recordDenied method calls within the
EntitlementGuard class (at the indicated line ranges including around line
80-90, 239-241, and 310), replace the fourth parameter that passes
`problem.detail ?? problem.message` with stable reason code values instead, such
as using `result.reason ?? "not_entitled"` or constant values like
`"provider_unavailable"` to ensure only predictable, low-cardinality codes are
recorded for telemetry.
- Line 6: The import statement in EntitlementGuard.ts is mixing a value import
(getEntitlementRequirements) with an inline type import (type
EntitlementRequirement) in a single import statement. According to the
repository's TypeScript guidelines, type imports must use the separate import
type syntax. Split the import statement into two separate imports from the same
module: one regular import statement for the value getEntitlementRequirements
and one import type statement for the EntitlementRequirement type.
In `@packages/entitlements-core/src/libs/EntitlementRequirement.ts`:
- Around line 121-129: The isEntitlementRequirement function currently only
validates the feature field but does not validate the resource field structure.
This can allow invalid resource objects (where type or other properties may not
be strings) to pass through to defineEntitlementRequirement, causing runtime
TypeErrors when accessing .length on lines 144, 150, and 156. Enhance the
validation in isEntitlementRequirement to also check that the resource field
exists and has a valid shape with all required string properties (such as type)
before returning true, ensuring that only properly structured
EntitlementRequirement objects pass validation and preventing downstream errors
in the resource field access.
In `@packages/protocols-core/src/libs/ContractGraphDiff.ts`:
- Around line 179-215: The function diffEntitlementRequirements has a bug where
duplicate changes are generated when entitlements with the same fingerprint
appear multiple times in the arrays. The issue is that while the function
creates Sets for membership checking (baselineEntitlements and
currentEntitlements), it then iterates over the original arrays
(current.entitlements and baseline.entitlements) which may contain duplicates.
This causes the same change to be added multiple times to the changes array. To
fix this, instead of iterating directly over current.entitlements and
baseline.entitlements, create deduplicated collections of entitlements by their
fingerprints and iterate over those deduplicated collections instead, ensuring
each unique entitlement fingerprint is only processed once.
In `@packages/protocols-core/src/tests/helpers/test-decorators.ts`:
- Around line 97-113: The RequiresEntitlement decorator currently overwrites
existing entitlement requirements instead of accumulating them when applied
multiple times to the same target. To fix this, retrieve any existing metadata
using Reflect.getMetadata with the same ENTITLEMENT_REQUIREMENTS_KEY,
propertyKey, and target before calling Reflect.defineMetadata in both the method
decorator case (when propertyKey is provided) and the class decorator case (when
it is not). If existing metadata is found, append the new requirement to that
array before defining the metadata; otherwise, create a new array with just the
current requirement. This ensures that multiple decorator applications
accumulate requirements rather than replacing them.
In `@public-api-surface.snapshot.json`:
- Around line 3423-3430: The entries for ENTITLEMENT_REQUIRED_KEY and
ENTITLEMENT_REQUIREMENTS_KEY in the snapshot file are missing the
declarationKind field that all other const exports contain. Add
"declarationKind": "const" to both of these entries to match the structure of
other const declarations in the snapshot and ensure API validation tools can
properly categorize these exports.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6bee7cfe-1b66-4dd9-9fc8-b6727b724f2e
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (31)
.changeset/entitlement-guard-contracts.mdexamples/saas-billing-golden-path/vitest.config.tspackages/admin-react/src/tests/AdminPanel.spec.tspackages/cli/src/tests/contractsCheck.spec.tspackages/cli/src/tests/contractsDiff.spec.tspackages/entitlements-core/README.mdpackages/entitlements-core/package.jsonpackages/entitlements-core/src/index.tspackages/entitlements-core/src/libs/EntitlementGuard.tspackages/entitlements-core/src/libs/EntitlementManager.tspackages/entitlements-core/src/libs/EntitlementRequirement.tspackages/entitlements-core/src/libs/decorators/RequireEntitlement.tspackages/entitlements-core/src/libs/interfaces.tspackages/entitlements-core/src/libs/problems/EntitlementProblems.tspackages/entitlements-core/src/libs/types.tspackages/entitlements-core/src/tests/EntitlementGuard.spec.tspackages/entitlements-core/src/tests/EntitlementIntegration.spec.tspackages/entitlements-core/src/tests/EntitlementManager.spec.tspackages/framework-routes/src/__tests__/compiler.spec.tspackages/openapi-spec/src/libs/emitOpenAPI.tspackages/openapi-spec/src/tests/emitOpenAPI.spec.tspackages/protocols-core/src/index.tspackages/protocols-core/src/libs/ContractGraph.tspackages/protocols-core/src/libs/ContractGraphConsumerCoverage.tspackages/protocols-core/src/libs/ContractGraphDiff.tspackages/protocols-core/src/libs/ContractGraphSnapshot.tspackages/protocols-core/src/libs/sharedTypes.tspackages/protocols-core/src/tests/ContractGraph.spec.tspackages/protocols-core/src/tests/helpers/test-decorators.tspackages/rpc-codegen/src/tests/codegen.spec.tspublic-api-surface.snapshot.json
kang-heewon
force-pushed
the
fix/920-entitlement-guard
branch
from
June 20, 2026 07:50
cfbbaf5 to
f222a24
Compare
kang-heewon
force-pushed
the
fix/920-entitlement-guard
branch
from
June 20, 2026 08:03
f222a24 to
f881684
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/entitlements-core/src/libs/EntitlementGuard.ts`:
- Around line 71-95: The createGuardInput method call at line 72 is executed
outside the try-catch block, which means any EntitlementDeniedProblem thrown
during input validation and preprocessing bypasses the recordDenied call that is
inside the catch block. This results in missing denied telemetry and audit logs.
Move the createGuardInput call inside the try block (before the
entitlementManager.check call) so that exceptions thrown during input creation
are also caught and properly recorded through the recordDenied method.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 37e716dc-c433-4d57-9350-68f8985ee80a
📒 Files selected for processing (20)
.changeset/entitlement-guard-contracts.mdexamples/saas-billing-golden-path/vitest.config.tspackages/admin-react/src/tests/AdminPanel.spec.tspackages/cli/src/tests/contractsCheck.spec.tspackages/cli/src/tests/contractsDiff.spec.tspackages/cli/vitest.config.tspackages/entitlements-core/README.mdpackages/entitlements-core/package.jsonpackages/entitlements-core/src/index.tspackages/entitlements-core/src/libs/EntitlementGuard.tspackages/entitlements-core/src/libs/EntitlementManager.tspackages/entitlements-core/src/libs/EntitlementRequirement.tspackages/entitlements-core/src/libs/decorators/RequireEntitlement.tspackages/entitlements-core/src/libs/interfaces.tspackages/entitlements-core/src/libs/problems/EntitlementProblems.tspackages/entitlements-core/src/libs/types.tspackages/entitlements-core/src/tests/EntitlementGuard.spec.tspackages/entitlements-core/src/tests/EntitlementIntegration.spec.tspackages/entitlements-core/src/tests/EntitlementManager.spec.tspackages/entitlements-core/src/tests/EntitlementRequirement.spec.ts
kang-heewon
force-pushed
the
fix/920-entitlement-guard
branch
from
June 21, 2026 04:29
1f3f51f to
488d6ff
Compare
kang-heewon
force-pushed
the
fix/920-entitlement-guard
branch
from
June 21, 2026 04:42
488d6ff to
99b7927
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
@RequireEntitlementmetadata toEntitlementGuard, including explicit tenant/user/resource/route guard input, status-aware Problems, telemetry, and optional audit sink evidence.x-croco-entitlements, with consumer coverage and diff gates for added/removed requirements.Closes #920.
Verification
pnpm checkpnpm typecheck --filter=@croco/entitlements-core --filter=@croco/protocols-core --filter=@croco/openapi-spec --filter=@croco/access-corepnpm test --filter=@croco/entitlements-core --filter=@croco/access-core --filter=@croco/protocols-core --filter=@croco/openapi-specpnpm changeset-required:check -- --base origin/trunk --head HEADgit diff --checkpnpm test && pnpm typecheckSelf-review gates
Risk
EntitlementCheckResult.statusis now required, so fixture and downstream compile surfaces were updated with explicit states.Summary by CodeRabbit
릴리스 노트
New Features
x-croco-entitlements확장 포함Documentation
Tests