feat(license): dual-read v1/v2 entitlements with discrepancy logging#6914
feat(license): dual-read v1/v2 entitlements with discrepancy logging#6914PrestigePvP wants to merge 2 commits into
Conversation
|
💬 Discussion in Slack: #pr-review-infisical-6914-feat-license-dual-read-v1-v2-entitlements-with-discrepa Posted by Review Police — reviews, comments, new commits, and CI failures will stream into this channel. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1b742f8a77
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
| Filename | Overview |
|---|---|
| backend/src/services/license-client/dual-read/dual-read-queue.ts | New BullMQ worker that fetches v2 entitlements and compares them against the v1 plan; getPlan/compareEntitlements calls are outside the error-capture try/catch, so unexpected failures won't be recorded in the error counter metric. |
| backend/src/services/license-client/license-client.ts | Migrates kill-switch from boolean LICENSE_SERVER_V2_ENABLED to tri-state LICENSE_SERVER_V2_MODE; exposes getEntitlements publicly; buildBackend lacks the HTTPS URL validation that buildUsageReporter enforces. |
| backend/src/services/license-client/dual-read/dual-read-service.ts | Debounced emit service using BullMQ jobId deduplication; fire-and-forget with proper catch; looks correct. |
| backend/src/services/license-client/dual-read/entitlement-comparator.ts | Pure comparator with correct truthy-object guard for v2Raw and UNLIMITED-XOR Indeterminate logic; all edge cases are covered by the unit tests. |
| backend/src/ee/services/license/license-service.ts | Adds optional skipDualReadEmit flag to getPlan cache-miss path; emit is correctly scoped to Cloud instance type and guarded against re-entrancy. |
| backend/src/lib/config/env.ts | Replaces boolean LICENSE_SERVER_V2_ENABLED with tri-state enum LICENSE_SERVER_V2_MODE; adds isLicenseDualReadEnabled computed property; clean migration. |
| backend/src/server/routes/index.ts | Wires up dualReadServiceFactory and dualReadQueueFactory in the correct order; dualReadQueue.init() added before healthAlert.init(); clean integration. |
Comments Outside Diff (1)
-
backend/src/services/license-client/license-client.ts, line 19-33 (link)SSRF: missing HTTPS check before bearer-token transmission
buildBackendpassesLICENSE_SERVER_V2_URLdirectly tolicenseServerBackendwithout validating that the scheme ishttps:. TheLICENSE_SERVER_V2_SERVICE_KEYbearer token is then sent in every entitlement request.buildUsageReporter(changed in this same PR) already enforces HTTPS:if (parsedUrl.protocol !== "https:") { logger.warn("usage-reporter: LICENSE_SERVER_V2_URL must use https; …"); return null; }read-compareis the first mode where this backend becomes live, so the gap is now exploitable: ifLICENSE_SERVER_V2_URLis misconfigured to an HTTP endpoint (or the URL is changed at the infrastructure level), the service key is transmitted in plaintext. Apply the same URL-parse + protocol guard here thatbuildUsageReporteruses.Context Used: Flag SSRF risks (source)
Reviews (1): Last reviewed commit: "feat(license): dual-read v1/v2 entitleme..." | Re-trigger Greptile
Addresses PR review feedback on #6914. - BullMQ throws "Custom Id cannot contain :" for a custom jobId with a single colon, so the dual-read job never enqueued; switch to the hyphen jobId convention used everywhere else so compares actually run - Wrap getEntitlements + getPlan + compareEntitlements in one try/catch so every failure increments infisical.license.dual_read.error.count
…e/on) Run cloud entitlement reads against License Server v2 alongside v1, gated by LICENSE_SERVER_V2_MODE so read-compare -> on is a config flip, not a code change. (PLATFOR-449) - Replace boolean LICENSE_SERVER_V2_ENABLED with tri-state LICENSE_SERVER_V2_MODE (off | read-compare | on); migrate v2 SDK + usage-metering gating onto it - getPlan is the toggle: off serves v1; read-compare serves v1 and shadow-compares v2 via the real client SDK (logs discrepancies + DataDog diff/error counters); on serves a v2 entitlement set projected into the TFeatureSet shape, so getPlan callers are unchanged - Data-driven v1<->v2 feature mapping over the full TFeatureSet + pure comparator (match/mismatch/v2_missing/v1_absent/indeterminate); fire-and-forget compare, no queue - Register license.feature / license.dual_read.kind in the InfisicalCore metric allowlist
… removed boolean Reconcile the billing-v2 surface (merged from main) with the new LICENSE_SERVER_V2_MODE, since this branch removed LICENSE_SERVER_V2_ENABLED. - license-v2-service isEnabled() now requires mode === "on" (every v2 billing route is gated on it), so the portal/checkout/overview surface is live only at full v2 cutover, not in read-compare - admin server-config licenseServerV2Enabled reflects mode === "on" so the v2 billing UI aligns with the backend gate - license-v2-service envConfig Pick moved to LICENSE_SERVER_V2_MODE
b92c16e to
8f46769
Compare
Context
Phase 2 of the License Server v2 migration.
getPlanbecomes the mode-aware read toggle driven byLICENSE_SERVER_V2_MODE(off | read-compare | on), so the cutover from shadow comparison to serving v2 is a config flip, not a code change. The read path uses the reallicenseClientSDK so v2 is exercised against real traffic before anything depends on it.offserves v1 (today's behavior).read-compareserves v1 and shadow-compares v2 in the background via the real SDK, logging discrepancies and emitting DataDog diff/error counters.onserves a v2 entitlement set projected into theTFeatureSetshape, sogetPlan's callers are untouched and flippingread-compare -> onneeds no code change.LICENSE_SERVER_V2_ENABLEDwith the tri-state mode and migrates the v2 SDK + usage-metering gating onto it. The old env var is removed (it was inert and never enabled in prod).projectV2ToFeatureSet: a generic v2 ->TFeatureSetprojection over a data-driven v1<->v2 feature-mapping registry covering the fullTFeatureSet. For features v2 omits or returns null for, the free-tier default is kept (the read-compare bake validates v2 completeness before the flip).compareInBackgroundoff thegetPlancache-miss with the already-resolved plan passed in (no re-fetch, no self-emit loop). Replaces the earlier BullMQ queue/worker.compareEntitlementsis pure and classifies each feature asmatch | mismatch | v2_missing | v1_absent | indeterminate(a present-but-null v2 value counts as missing;nullcaps normalize to "unlimited").license.feature/license.dual_read.kindin the InfisicalCore meter allowlist so the diff counter keeps its dimensions, and emits on Victor's low-cardinalityinfisicalCoreMeter(not the legacy self-host meter).The v2 feature keys are best-effort snake_case and must be reconciled with the License Server v2 feature registry; a wrong key surfaces as a
v2_missingdiff, which is intended bake signal. Lossy/non-entitlement fields the bake does not validate (slug, tier, usage counts) are projected on a best-effort basis; usage counts continue to be computed from the DB.Linear: https://linear.app/infisical/issue/PLATFOR-449
Steps to verify the change
cd backend && npm run test:unit -- src/services/license-client-> 40 passed (comparator incl. null-handling, v2->TFeatureSet projection, mapping-completeness over the full TFeatureSet, migrated SDK/metering tests)LICENSE_SERVER_V2_MODE=read-compareagainst a v2 backend: trigger a cloudgetPlan, confirmlicense-dual-read discrepancylogs + diff/error counters;LICENSE_SERVER_V2_MODE=on: confirm getPlan serves the v2-projected plan;off: neither path runsType
Checklist