fix: verify signed online license entitlements#1442
Conversation
|
@brendan-kellam your pull request is missing a changelog! |
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 docstrings
🧪 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}
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
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 6fdad4f. Configure here.
| // Unsigned database columns never grant online entitlements. | ||
| if (!_license?.licenseAssertion) { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
Unsigned rollout compatibility missing
High Severity
getValidOnlineLicense returns null whenever licenseAssertion is absent, with no legacy unsigned path. The PR describes retaining ALLOW_LEGACY_UNSIGNED_ONLINE_LICENSES for initial rollout, but that flag is not implemented anywhere in this repository, so existing rows with active mutable license data and a null assertion lose all online entitlements immediately after upgrade until a signed assertion is persisted.
Reviewed by Cursor Bugbot for commit 6fdad4f. Configure here.
| startChangelogPollingJob(); | ||
| warmModelCapabilitiesCatalog(); | ||
| })(); | ||
| }; |
There was a problem hiding this comment.
Search context wipe removed
Medium Severity
Refactoring initialize removed the startup check that deleted org search contexts when the search-contexts entitlement was missing. With stricter assertion-based entitlements, revoked customers can keep persisted contexts and EE chat can still expand reposet scopes from the database without that entitlement.
Reviewed by Cursor Bugbot for commit 6fdad4f. Configure here.
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
1 participant
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 EE authorization: invalid or missing assertions strip entitlements immediately once enforcement applies, and service ping failures can block persisting license updates—coordinate rollout with the Lighthouse signer companion PR.
Overview
Online EE licensing now fails closed on unsigned or tampered state. Paid features and anonymous-access gating no longer trust mutable
Licensecolumns (status,entitlements,lastSyncAt); they require a persisted Lighthouse-signedlicenseAssertionwhose claims are verified (signature, audience,installId, issuance/expiry, max lifetime, and license snapshot schema).Service ping verifies any returned assertion before writing it (and refuses assertion-without-license or invalid signatures so legacy fields cannot be used as a downgrade path). License rows are updated from the verified snapshot, not raw response fields alone.
Shared
lighthouseTypesnow defines the online license snapshot and ping response shape (including optionallicenseAssertion); web billing imports these instead of local duplicates. Startup runs an initial Lighthouse sync before the daily ping cron (errors logged to Sentry, startup continues).Extensive entitlements tests cover claim validation and the no-fallback-to-DB-columns behavior.
Reviewed by Cursor Bugbot for commit 6fdad4f. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
Bug Fixes
Tests