feat(security): RS256 entitlement verification for premium license enforcement (#406)#443
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughIntroduces ChangesPremium License Entitlement Enforcement
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related issues
Poem
🚥 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 |
🧪 PR Test Results
Python 3.12 · commit ed6055b |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/test_license.py (1)
122-133: ⚡ Quick winAdd malformed-claim type regression tests.
Coverage is strong, but there’s no case for wrong claim types (e.g.,
features=123orexp="tomorrow"). Add assertions that these reject withLicenseInvalidErrorto protect the error-contract behavior.Also applies to: 142-151
🤖 Prompt for 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. In `@tests/test_license.py` around lines 122 - 133, Add additional test cases to cover malformed claim types beyond just missing claims. Create new test functions that verify the verify_entitlement function properly rejects tokens with incorrect claim types, such as features being an integer instead of the expected type, or exp being a string like "tomorrow" instead of a proper timestamp. Each test should follow the same pattern as test_missing_required_claim_rejected by creating a token with the malformed claim type and asserting that verify_entitlement raises LicenseInvalidError with the public key.
🤖 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 `@agentwatch/security/license.py`:
- Around line 133-139: Before constructing the Entitlement object, add type
validation for the claims that are assumed to have specific types. Validate that
claims["exp"] is a numeric timestamp (int or float) before passing it to
datetime.fromtimestamp, and validate that claims["features"] is an iterable of
strings before passing it to frozenset. If either validation fails, raise
LicenseInvalidError instead of allowing the underlying type error to propagate
uncaught. This ensures the function maintains its typed error contract and
provides clear validation failures at the boundary.
---
Nitpick comments:
In `@tests/test_license.py`:
- Around line 122-133: Add additional test cases to cover malformed claim types
beyond just missing claims. Create new test functions that verify the
verify_entitlement function properly rejects tokens with incorrect claim types,
such as features being an integer instead of the expected type, or exp being a
string like "tomorrow" instead of a proper timestamp. Each test should follow
the same pattern as test_missing_required_claim_rejected by creating a token
with the malformed claim type and asserting that verify_entitlement raises
LicenseInvalidError with the public key.
🪄 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: CHILL
Plan: Pro Plus
Run ID: bd9f2d61-1882-4df4-9aa3-3c0b621ec4b2
📒 Files selected for processing (3)
agentwatch/security/license.pypyproject.tomltests/test_license.py
…lement (sreerevanth#406) Guard the 'sub'/'tier'/'features' claims so a malformed token raises LicenseInvalidError at the boundary instead of an uncaught TypeError, preserving the typed error contract. (exp is already validated as numeric by PyJWT during decode.) Adds regression tests for non-numeric exp, non-string tier, and non-iterable features. Addresses CodeRabbit review feedback on sreerevanth#443. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…forcement (sreerevanth#406) Add the client-side verification primitive for premium entitlements: a JWT signed by the backend with an RS256 private key, verified by the CLI against the matching public key. Asymmetric signing means the client holds only the public key and cannot mint its own tokens, so premium gating becomes a signature check rather than a patchable boolean. - verify_entitlement(): RS256-only decode, required claims, expiry, and optional machine binding; fails closed if PyJWT is unavailable. - current_machine_id(): stable, hashed device fingerprint for binding. - require_feature(): entitlement-gated feature check (no bare boolean). - pyjwt added to the optional `crypto` extra. Scope: client verification + device binding only. Server-side enforcement and token issuance are tracked in sreerevanth#405. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…lement (sreerevanth#406) Guard the 'sub'/'tier'/'features' claims so a malformed token raises LicenseInvalidError at the boundary instead of an uncaught TypeError, preserving the typed error contract. (exp is already validated as numeric by PyJWT during decode.) Adds regression tests for non-numeric exp, non-string tier, and non-iterable features. Addresses CodeRabbit review feedback on sreerevanth#443. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
c0aa682 to
ed6055b
Compare
…lement (sreerevanth#406) Guard the 'sub'/'tier'/'features' claims so a malformed token raises LicenseInvalidError at the boundary instead of an uncaught TypeError, preserving the typed error contract. (exp is already validated as numeric by PyJWT during decode.) Adds regression tests for non-numeric exp, non-string tier, and non-iterable features. Addresses CodeRabbit review feedback on sreerevanth#443. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Closes part of #406 (client-side verification slice).
What
Adds the client-side verification primitive for premium entitlements. A premium entitlement is a JWT signed by the backend with an RS256 private key and verified by the CLI against the matching public key. Because signing is asymmetric, the client only ever holds the public key and cannot mint its own tokens — premium gating becomes a signature check rather than a patchable
if is_premium()boolean.Changes
agentwatch/security/license.pyverify_entitlement(token, public_key, *, machine_id=None)— RS256-only decode, mandatorysub/exp/tierclaims, expiry enforcement, and optional device binding. Fails closed (LicenseUnavailableError) when PyJWT is absent rather than failing open.current_machine_id()— stable, hashed device fingerprint for binding tokens to a machine (enables the backend to flag concurrent use across unlinked devices).require_feature(entitlement, feature)— entitlement-gated feature check; no bare boolean to strip.LicenseError.pyjwtadded to the optionalcryptoextra (cryptographyalready present).Acceptance criteria mapping (#406)
machine_idclaim +current_machine_id()).Test
🤖 Generated with Claude Code
Summary by CodeRabbit