fix: verify signed online license entitlements#1442
Merged
Conversation
This comment has been minimized.
This comment has been minimized.
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThe change adds signed online license assertions to the license schema and Lighthouse sync flow, verifies assertion claims before deriving entitlements, updates shared billing contracts, and makes startup explicitly await license synchronization before starting background jobs. ChangesOnline license assertion flow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Lighthouse
participant syncWithLighthouse
participant verifyOnlineLicenseAssertion
participant PrismaLicense
participant getEntitlements
Lighthouse->>syncWithLighthouse: signed license assertion
syncWithLighthouse->>verifyOnlineLicenseAssertion: verify assertion
verifyOnlineLicenseAssertion-->>syncWithLighthouse: validated claims
syncWithLighthouse->>PrismaLicense: persist assertion
PrismaLicense->>getEntitlements: stored license
getEntitlements->>verifyOnlineLicenseAssertion: validate stored assertion
verifyOnlineLicenseAssertion-->>getEntitlements: active entitlements
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/shared/src/entitlements.ts`:
- Around line 61-62: Update the online assertion persistence and validation flow
around the entitlement schema fields licenseId and installId to retain the
upstream licenseId with each persisted assertion, and require it to match the
current License row before accepting the assertion. Add a test covering reuse of
a valid assertion across different License rows on the same installation,
ensuring the cross-license replay is rejected.
In `@packages/web/src/features/billing/servicePing.ts`:
- Around line 129-136: Update the service-ping handling around
verifyOnlineLicenseAssertion so assertion presence is checked explicitly and
validated before the response.license synchronization block, rejecting empty or
invalid assertions. Handle assertion-only responses independently: persist them
when supported, or reject the response as malformed rather than skipping
persistence and leaving stale entitlements active; apply the same behavior to
the corresponding later handling.
🪄 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
Run ID: 68eac570-e897-4f14-b103-bf840cd35d86
📒 Files selected for processing (9)
CHANGELOG.mdpackages/db/prisma/migrations/20260713000000_add_license_assertion/migration.sqlpackages/db/prisma/schema.prismapackages/shared/src/entitlements.test.tspackages/shared/src/entitlements.tspackages/shared/src/index.server.tspackages/web/src/app/(app)/components/banners/bannerResolver.test.tspackages/web/src/features/billing/servicePing.tspackages/web/src/features/billing/types.ts
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/shared/src/entitlements.ts (1)
182-206: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftAssertion isn't bound to the persisted License row it's being validated against, and the rollout legacy-flag fallback isn't visible here.
Two concerns in
getValidOnlineLicense:
- Per a past review round on this PR, reusing a valid assertion across different
Licenserows on the same installation (since onlyinstallIdis checked) was flagged and marked "Addressed", requesting that the upstreamlicenseIdbe persisted and compared before accepting an assertion. The code shown here still only checkspayload.installId !== env.SOURCEBOT_INSTALL_IDinsideverifyOnlineLicenseAssertion(Line 93) — there's no comparison ofassertion.licenseIdagainst any identifier on_licenseingetValidOnlineLicense(Lines 187-203), andentitlements.test.tshas no cross-license replay test. Please confirm whether the binding is enforced elsewhere (e.g. a DB-level unique constraint guaranteeing oneLicenserow per install) or whether this gap is still open.- PR objectives state an unsigned compatibility path remains available via
ALLOW_LEGACY_UNSIGNED_ONLINE_LICENSESfor the initial rollout, but this function returnsnullunconditionally wheneverlicenseAssertionis absent (Lines 189-191), with no reference to that flag. If this fallback is meant to live here, licenses that haven't yet received a signed assertion would lose all online entitlements the moment this code ships, even with the flag enabled.🔍 Verification script
#!/bin/bash rg -n -C5 '\blicenseId\b' packages/shared/src/entitlements.ts packages/shared/src/entitlements.test.ts packages/db/prisma/schema.prisma rg -n -C5 'ALLOW_LEGACY_UNSIGNED_ONLINE_LICENSES' packages🤖 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 `@packages/shared/src/entitlements.ts` around lines 182 - 206, Update getValidOnlineLicense to bind the verified assertion’s licenseId to the persisted License row before accepting entitlements, using the existing database field or enforce the documented one-row-per-install invariant if that is the established contract. Add the ALLOW_LEGACY_UNSIGNED_ONLINE_LICENSES fallback for licenses without licenseAssertion, preserving unsigned entitlements only when the flag is enabled. Add coverage in entitlements.test.ts for cross-license assertion replay and the enabled/disabled unsigned-license behavior.
🧹 Nitpick comments (1)
packages/shared/src/entitlements.ts (1)
66-113: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueMinor: date parsing failure silently passes validation.
If
payload.issuedAt/expiresAtfail to parse,new Date(...).getTime()returnsNaN, and every comparison in the OR-chain (Lines 93-102) involvingNaNevaluates tofalse— so the whole guard would befalseand the function would fall through toreturn payloadinstead of rejecting. This is presumably masked today becauseonlineLicenseAssertionClaimsSchema.parse(inlighthouseTypes.ts, not in this batch) likely enforces strict ISO datetime strings before this code runs — the testrejects an invalid issuedAt timestampimplies this. Still, adding an explicitNumber.isNaN(issuedAt) || Number.isNaN(expiresAt)check here would make the invariant self-evident and not solely dependent on an external schema.🛡️ Optional defensive check
const issuedAt = new Date(payload.issuedAt).getTime(); const expiresAt = new Date(payload.expiresAt).getTime(); const now = Date.now(); // A license is considered invalid under the following // circumstances: if ( + // 0. Its timestamps failed to parse. + Number.isNaN(issuedAt) || Number.isNaN(expiresAt) || // 1. It was issued for a different Sourcebot installation. payload.installId !== env.SOURCEBOT_INSTALL_ID ||🤖 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 `@packages/shared/src/entitlements.ts` around lines 66 - 113, Update verifyOnlineLicenseAssertion to explicitly reject invalid date parsing by checking Number.isNaN(issuedAt) or Number.isNaN(expiresAt) in the license-claims validation guard before the existing timestamp comparisons. Preserve the current rejection and logging behavior for invalid claims.
🤖 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.
Outside diff comments:
In `@packages/shared/src/entitlements.ts`:
- Around line 182-206: Update getValidOnlineLicense to bind the verified
assertion’s licenseId to the persisted License row before accepting
entitlements, using the existing database field or enforce the documented
one-row-per-install invariant if that is the established contract. Add the
ALLOW_LEGACY_UNSIGNED_ONLINE_LICENSES fallback for licenses without
licenseAssertion, preserving unsigned entitlements only when the flag is
enabled. Add coverage in entitlements.test.ts for cross-license assertion replay
and the enabled/disabled unsigned-license behavior.
---
Nitpick comments:
In `@packages/shared/src/entitlements.ts`:
- Around line 66-113: Update verifyOnlineLicenseAssertion to explicitly reject
invalid date parsing by checking Number.isNaN(issuedAt) or
Number.isNaN(expiresAt) in the license-claims validation guard before the
existing timestamp comparisons. Preserve the current rejection and logging
behavior for invalid claims.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2e611b56-5878-4926-a773-424ec7f44ba7
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (21)
packages/backend/package.jsonpackages/db/prisma/schema.prismapackages/shared/package.jsonpackages/shared/src/entitlements.test.tspackages/shared/src/entitlements.tspackages/shared/src/index.client.tspackages/shared/src/index.server.tspackages/shared/src/lighthouseTypes.tspackages/web/src/app/(app)/settings/license/recentInvoicesCard.tsxpackages/web/src/app/(app)/settings/license/types.tspackages/web/src/app/api/(client)/client.tspackages/web/src/ee/features/lighthouse/actions.tspackages/web/src/features/billing/CLAUDE.mdpackages/web/src/features/billing/client.tspackages/web/src/features/billing/planComparisonTable.tsxpackages/web/src/features/billing/servicePing.tspackages/web/src/features/billing/systemInfo.tspackages/web/src/features/billing/upsellDialog.tsxpackages/web/src/initialize.test.tspackages/web/src/initialize.tspackages/web/src/instrumentation.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/db/prisma/schema.prisma
- packages/web/src/features/billing/servicePing.ts
Contributor
License Audit
Weak Copyleft Packages (informational)
Resolved Packages (17)
|
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit a672a09. Configure here.
| return { | ||
| entitlements: assertion.license.entitlements.filter(isKnownEntitlement), | ||
| status: assertion.license.status, | ||
| }; |
There was a problem hiding this comment.
Staleness banners mismatch assertions
Medium Severity
The new online license enforcement relies on the signed assertion's expiresAt, but staleness banners still use lastSyncAt. This mismatch can cause the UI to incorrectly show an enforced outage while entitlements are active, or fail to warn when entitlements have actually expired.
Reviewed by Cursor Bugbot for commit a672a09. Configure here.
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.
Fixes SOU-1465
Companion Lighthouse PR: https://github.com/sourcebot-dev/lighthouse/pull/27
Summary
Rollout
Deploy the Lighthouse signer first. After it has issued assertions for at least one seven-day assertion lifetime, disable
ALLOW_LEGACY_UNSIGNED_ONLINE_LICENSESin the enforcement release.Testing
yarn build:depsyarn workspace @sourcebot/shared test --run src/entitlements.test.tsyarn workspace @sourcebot/web test --run src/app/\(app\)/components/banners/bannerResolver.test.tsNote
High Risk
Changes core licensing and entitlement enforcement; misconfiguration or Lighthouse rollout timing could strip paid features until assertions sync successfully.
Overview
Online EE licensing now requires a cryptographically verified Lighthouse assertion to grant paid features, instead of trusting mutable
Licensecolumns (status,entitlements,lastSyncAt).The PR adds
licenseAssertiononLicense, implementsverifyOnlineLicenseAssertion(signature, audience/version,installId, issuance/expiry, max 7-day lifetime), and changesgetValidOnlineLicenseso unsigned rows never grant online entitlements. Entitlements and active status come from the signed snapshot; invalid or missing assertions fail closed with no fallback to DB fields.Service ping verifies any returned assertion before persistence, writes license fields from the verified payload, and errors if Lighthouse sends a bad assertion or assertion without license data. Startup runs an initial
syncWithLighthousebefore the daily ping cron (failures are logged/Sentry but do not block boot).Lighthouse Zod schemas (
onlineLicenseSnapshotSchema, assertion claims, ping response with optionallicenseAssertion) move intopackages/shared/src/lighthouseTypes.ts; web billing imports shift to@sourcebot/shared. Offline license keys keep prior behavior.Reviewed by Cursor Bugbot for commit a672a09. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit