feat: license validation #1417
feat: license validation #1417
Conversation
|
Note
|
| Cohort / File(s) | Change Summary |
|---|---|
License Type Definitions packages/types/src/license.ts, packages/types/src/index.ts |
Added TypeScript types for license validation: deployment provider, subscription types (free/starter/growth/enterprise), request/response payloads for license checks, and service response types. |
Backend License Configuration packages/server/.env.example, packages/server/src/constants.ts, packages/server/src/environment.d.ts, packages/server/src/utils/logchimpConfig.ts |
Added license environment variables (LOGCHIMP_LICENSE_KEY, LOGCHIMP_SIGNATURE_TOKEN, LOGCHIMP_PILOT_URL), cache TTL constant, TypeScript type declarations for env vars, and extended configuration merging to include license fields and version info. |
Backend License Controller & Route packages/server/src/ee/controllers/v1/checkLicense.ts, packages/server/src/ee/routes/v1/license.ts |
Added new license validation controller that checks license status and returns 403 if invalid or 200 if active; wired controller to GET /license route. |
License Guard Middleware Integration packages/server/src/ee/routes/v1/boards.ts, packages/server/src/ee/routes/v1/posts.ts, packages/server/src/ee/routes/v1/roadmaps.ts, packages/server/src/ee/routes/v1/roles.ts |
Applied withLicenseGuard middleware to feature routes, enforcing required subscription tiers (starter, growth, enterprise for most features; enterprise for roles). |
License Error Messages packages/server/src/errorResponse.json, packages/server/src/routes/v1/index.ts |
Added license verification and higher plan error responses; integrated license route into v1 API router. |
Request Type Refactoring packages/server/src/express.d.ts, packages/server/src/middlewares/roadmapExists.ts, packages/server/src/routes/v1/roadmaps.ts, packages/server/src/controllers/roadmaps/roadmapByUrl.ts, packages/server/src/ee/controllers/v1/roadmaps/updateRoadmap.ts |
Removed custom ExpressRequestContext type usage, replaced with standard Express Request type; updated roadmap property access from req.ctx.roadmap to req.roadmap with TypeScript error suppression. |
Submodule Registration .gitmodules, .idea/vcs.xml, packages/server/src/ee/do-not-remove |
Added git submodule for logchimp-ee (license guard middleware implementation). |
Backend Settings Store packages/server/src/controllers/settings/siteSettings.ts, packages/theme/src/store/settings.ts, packages/types/src/site/settings.ts |
Added hasValidLicense boolean field to site settings; updated settings controller response structure to wrap settings in a container object. |
Frontend License Configuration docker/community/docker-compose.yml, packages/theme/.env.example, packages/theme/inject-app-env.sh, packages/theme/src/constants.ts |
Added VITE_WEBAPP_URL environment variable to Docker, .env, and script injection; added IS_DEV, IS_PROD, and DEFAULT_LOGCHIMP_PILOT_URL constants. |
Frontend License Components packages/theme/src/components/EmptyScreen.vue, packages/theme/src/ee/components/LicenseRequired.vue, packages/theme/src/ee/components/UpgradeTooltip.vue, packages/theme/src/ee/components/license/LocalLicenseAlert.vue, packages/theme/src/ee/components/icons/LicenseCrown.vue |
Added reusable Vue components for empty states, license gating, upgrade tooltips with sales links, license alerts, and crown icon indicator. |
Frontend License Service & Store packages/theme/src/ee/modules/settings.ts, packages/theme/src/ee/store/settings.ts |
Added SettingsEE service class with checkLicense API call; added useSettingsEEStore with throttled getLicenseInfo function that updates hasValidLicense reactively based on license validation response. |
Feature Pages with License Gates packages/theme/src/ee/pages/dashboard/boards/Index.vue, packages/theme/src/ee/pages/dashboard/roadmaps/Index.vue, packages/theme/src/ee/pages/dashboard/settings/roles/Index.vue, packages/theme/src/ee/pages/dashboard/settings/billing.vue, packages/theme/src/components/dashboard/posts/PostEditor/index.vue |
Wrapped feature creation buttons and list views with UpgradeTooltip and LicenseRequired gates; added LicenseCrown icons; integrated license store and onMounted license info fetching. |
Core Posts Page Update packages/theme/src/pages/dashboard/posts/Index.vue |
Removed PhCrownSimple icon from Create Post button. |
Dropdown Styling Updates packages/theme/src/ee/components/dashboard/boards/SearchBoardDropdown/Dropdown.vue, packages/theme/src/ee/components/dashboard/roadmap/SearchRoadmapDropdown/Dropdown.vue |
Added disabled state styling (bg, cursor, text color, icon stroke) to dropdown triggers. |
Sequence Diagram(s)
sequenceDiagram
participant User
participant Frontend as Frontend App
participant API as Backend API
participant LicenseService as License Service
rect rgb(200, 220, 255)
Note over Frontend,API: On Component Mount - License Info Fetch
Frontend->>API: GET /v1/license
API->>LicenseService: Decrypt & validate license
LicenseService-->>API: ICheckLicenseDecryptedPayload
API-->>Frontend: License status
Frontend->>Frontend: hasValidLicense = (status === 'active')
end
rect rgb(220, 255, 200)
Note over User,Frontend: Feature Access - Licensed Feature
User->>Frontend: Click "Create Board" button
alt hasValidLicense === true
Frontend->>Frontend: Render create form
Frontend->>API: POST /v1/boards
API->>LicenseGuard: Check license tier
LicenseGuard-->>API: Valid (starter+)
API->>API: Process request
API-->>Frontend: 200 Created
else hasValidLicense === false
Frontend->>Frontend: Show UpgradeTooltip
Frontend->>Frontend: Disable button
end
end
rect rgb(255, 220, 200)
Note over User,API: Backend License Guard Check
User->>API: POST /v1/roadmaps
API->>LicenseGuard: withLicenseGuard (requiredPlan: [starter, growth, enterprise])
LicenseGuard->>LicenseGuard: Validate subscription tier in license
alt Tier match
LicenseGuard-->>API: Proceed to handler
API-->>User: 200 Resource created
else Tier mismatch or no license
LicenseGuard-->>User: 403 Need higher plan
end
end
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~45 minutes
- Heterogeneous changes: Mix of new types, services, components, middleware integrations, and structural refactoring across multiple packages
- Logic density: License validation logic, throttled store fetching, middleware guards, and conditional UI rendering require understanding of licensing business rules
- Scope: ~35 files changed across backend, frontend, types, and configuration; new submodule integration
- Areas requiring extra attention:
- License guard middleware implementation (in submodule) and its integration across 4 route files to ensure required tiers are correctly enforced
- Request type refactoring from
ExpressRequestContextto standardRequest; verify all property accesses (req.roadmap vs req.ctx.roadmap) are correctly updated - Frontend license store throttling logic and error handling (LICENSE_VALIDATION_FAILED code check)
- Integration of license checks in onMounted hooks across multiple page components to ensure no race conditions
- Settings controller response structure change (wrapping settings in container object) and impact on API consumers
Possibly related PRs
- Fix(update) post board roadmap #1358: Modifies
packages/server/src/ee/controllers/v1/roadmaps/updateRoadmap.tswith valibot validation; shares request type refactoring changes in the same file.
Suggested labels
High (P2)
Pre-merge checks and finishing touches
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. | You can run @coderabbitai generate docstrings to improve docstring coverage. |
✅ Passed checks (2 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title 'feat: license validation' directly and accurately describes the main change of the pull request, which comprehensively adds license validation infrastructure across the backend and frontend. |
✨ Finishing touches
- 📝 Generate docstrings
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
- Commit unit tests in branch
feat/license-validation
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 @coderabbitai help to get the list of available commands and usage tips.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (7)
packages/theme/src/ee/components/license/LocalLicenseAlert.vue (1)
12-12: Consider externalizing the support email.The email address
[email protected]is hardcoded in the template. Consider moving this to a constant for easier maintenance and consistency across the application.packages/theme/src/ee/pages/dashboard/settings/roles/Index.vue (1)
9-24: Consider disabling the button when license is invalid in production.The
Createbutton is wrapped inUpgradeTooltipbut remains functionally enabled regardless of license status. If the backend enforces license checks on thecreateRole()API, users without valid licenses will encounter errors after clicking. Consider adding:disabled="!settingsStore.settings.hasValidLicense"to the Button component in production environments to prevent this confusing experience.<Button type="primary" :loading="createRoleButtonLoading" - :disabled="createRoleButtonDisabled" + :disabled="createRoleButtonDisabled || (!settingsStore.settings.hasValidLicense && IS_PROD)" @click="createRoleHandler" >Note: You'll need to import
IS_PRODfrom constants if not already available.packages/theme/src/components/EmptyScreen.vue (1)
2-8: Strengthen type safety for the icon prop.The
iconprop is typed asunknown, which bypasses TypeScript's type checking when passed to thecomponent :isdirective. Consider using a more specific type likeComponentfrom Vue.<script setup lang="ts"> +import type { Component } from "vue"; + interface Props { title: string; - icon?: unknown; + icon?: Component; description?: string; border?: boolean; dashedBorder?: boolean; }packages/server/src/ee/services/license/checkLicense.ts (2)
22-22: Remove commented-out code.Clean up the commented code on line 22.
- // let cachedData: unknown; if (cache.isActive) {
40-47: Consider adding cache error handling.Cache operations could fail in production. While the current flow would still work (returning the status), logging cache failures would aid debugging.
if (cache.isActive) { - await cache.valkey.set( - getLogChimpCacheKey(licenseKey), - JSON.stringify(data), - "EX", - LOGCHIMP_LICENSE_CACHE_TTL_IN_SECONDS, - ); + try { + await cache.valkey.set( + getLogChimpCacheKey(licenseKey), + JSON.stringify(data), + "EX", + LOGCHIMP_LICENSE_CACHE_TTL_IN_SECONDS, + ); + } catch (error) { + console.error("Failed to cache license check result:", error); + } }packages/theme/src/ee/components/LicenseRequired.vue (1)
24-27: Document the development mode bypass behavior.In development mode, the component renders the slot content even without a valid license (after showing an alert). While this aids local development, it may cause confusion or runtime errors if backend endpoints enforce license checks. Consider adding a comment to explain this intentional bypass.
+ <!-- Development mode: show alert but allow access for local testing --> <div v-else-if="IS_DEV"> <LocalLicenseAlert class="mb-4" /> <slot /> </div>packages/server/src/utils/logchimpConfig.ts (1)
221-236: Remove commented-out helper method.The
getEnvhelper method is commented out and unused. Remove it to keep the codebase clean, or implement and use it if the functionality is needed.- // private getEnv = <K extends keyof NodeJS.ProcessEnv>(key: K, fallback?: NodeJS.ProcessEnv[K]): NodeJS.ProcessEnv[K] => { - // const value = process.env[key] as NodeJS.ProcessEnv[K] | undefined; - // - // if (value === undefined) { - // // handle fallback falsy cases that should still be used as value - // if (fallback === false || fallback === "" || fallback === 0) { - // return fallback; - // } - // if (fallback) { - // return fallback; - // } - // throw new Error(`Missing environment variable: ${key}.`); - // } - // - // return value; - // };
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
docker/community/docker-compose.yml(1 hunks)packages/server/.env.example(1 hunks)packages/server/src/constants.ts(1 hunks)packages/server/src/ee/routes/v1/license.ts(1 hunks)packages/server/src/ee/services/license/checkLicense.ts(1 hunks)packages/server/src/environment.d.ts(1 hunks)packages/server/src/utils/logchimpConfig.ts(8 hunks)packages/theme/.env.example(1 hunks)packages/theme/inject-app-env.sh(1 hunks)packages/theme/src/components/EmptyScreen.vue(1 hunks)packages/theme/src/constants.ts(2 hunks)packages/theme/src/ee/components/LicenseRequired.vue(1 hunks)packages/theme/src/ee/components/UpgradeTooltip.vue(1 hunks)packages/theme/src/ee/components/license/LocalLicenseAlert.vue(1 hunks)packages/theme/src/ee/pages/dashboard/boards/Index.vue(2 hunks)packages/theme/src/ee/pages/dashboard/roadmaps/Index.vue(2 hunks)packages/theme/src/ee/pages/dashboard/settings/billing.vue(2 hunks)packages/theme/src/ee/pages/dashboard/settings/roles/Index.vue(2 hunks)packages/theme/src/pages/dashboard/posts/Index.vue(3 hunks)packages/theme/src/store/settings.ts(1 hunks)packages/types/src/index.ts(1 hunks)packages/types/src/license.ts(1 hunks)packages/types/src/site/settings.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
packages/server/src/ee/services/license/checkLicense.ts (3)
packages/server/src/utils/logchimpConfig.ts (1)
configManager(239-239)packages/types/src/license.ts (2)
ICheckLicenseResponseBody(7-11)ICheckLicenseRequestBody(1-5)packages/server/src/constants.ts (1)
LOGCHIMP_LICENSE_CACHE_TTL_IN_SECONDS(10-10)
packages/server/src/utils/logchimpConfig.ts (1)
packages/theme/src/constants.ts (1)
DEFAULT_LOGCHIMP_PILOT_URL(15-16)
🪛 dotenv-linter (3.3.0)
packages/server/.env.example
[warning] 15-15: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 19-19: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 20-20: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 20-20: [UnorderedKey] The LOGCHIMP_PILOT_URL key should go before the LOGCHIMP_SIGNATURE_TOKEN key
(UnorderedKey)
packages/theme/.env.example
[warning] 2-2: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
🪛 GitHub Actions: Lighthouse test
packages/server/src/ee/routes/v1/license.ts
[error] 6-6: Cannot find module '../../controllers/v1/license' or its corresponding type declarations. This occurred during the build step: 'pnpm --filter="@logchimp/types" build; pnpm --filter="@logchimp/api" exec pnpm tsc;'.
🪛 GitHub Actions: Playwright Tests
packages/server/src/ee/routes/v1/license.ts
[error] 6-6: Cannot find module '../../controllers/v1/license' or its corresponding type declarations. (TS2307). Command: 'pnpm --filter="@logchimp/api" exec pnpm tsc'
🪛 GitHub Actions: Test
packages/server/src/ee/routes/v1/license.ts
[error] 6-6: Cannot find module '../../controllers/v1/license' or its corresponding type declarations. (TS2307) — Command failed: 'pnpm tsc --noEmit' during 'pnpm --filter @logchimp/api exec pnpm tsc --noEmit'
🪛 Gitleaks (8.28.0)
packages/server/.env.example
[high] 19-20: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (25)
docker/community/docker-compose.yml (1)
77-77: LGTM!The environment variable mapping is consistent with the existing pattern and correctly sources from
LOGCHIMP_WEB_URL.packages/theme/src/store/settings.ts (1)
16-16: LGTM!The
hasValidLicensefield with a default value offalsefollows a safe, pessimistic approach for feature gating.packages/theme/.env.example (1)
2-2: LGTM!The default value is consistent with the theme's default port and follows the existing pattern for VITE environment variables.
packages/types/src/index.ts (1)
10-10: LGTM!The barrel export follows the established pattern and makes license types accessible to consumers.
packages/types/src/site/settings.ts (1)
11-11: LGTM!The
hasValidLicensefield is appropriately typed as boolean and correctly omitted fromIUpdateSiteSettingsRequestBodysince it should be system-managed rather than user-editable.packages/theme/inject-app-env.sh (1)
8-8: LGTM!The environment variable injection follows the established pattern and correctly uses shell variable interpolation.
packages/server/src/constants.ts (1)
8-10: LGTM!The license cache TTL constant is correctly defined and the calculation (86,400 seconds = 1 day) matches the comment.
packages/theme/src/ee/pages/dashboard/roadmaps/Index.vue (2)
11-26: LGTM!The license gating UI is properly implemented with the UpgradeTooltip wrapper and conditional crown icon. The pattern aligns with similar implementations across other dashboard pages.
30-32: LGTM!Wrapping the TabularView with LicenseRequired properly gates the roadmap list behind license validation.
packages/theme/src/constants.ts (2)
1-2: LGTM!The IS_DEV and IS_PROD constants are correctly derived from Vite's built-in environment variables.
15-16: LGTM!The default pilot URL constant provides a consistent reference for license-related links across the UI.
packages/theme/src/ee/pages/dashboard/boards/Index.vue (2)
9-24: LGTM!The license gating implementation for the Create Board button follows the same pattern as other dashboard pages, providing consistent UX.
28-30: LGTM!Properly gates the boards table behind license validation using LicenseRequired.
packages/theme/src/pages/dashboard/posts/Index.vue (2)
9-24: LGTM!The license gating UI for the Create Post button is properly implemented with UpgradeTooltip and the conditional crown icon.
27-39: Verify the intentional omission of LicenseRequired wrapper.Unlike the boards and roadmaps pages, the posts list is not wrapped with a LicenseRequired component. This means users can view posts without a valid license but cannot create them. Please confirm this is the intended behavior for the licensing model.
packages/server/src/environment.d.ts (1)
16-18: LGTM!The license-related environment variable declarations are properly typed and follow the existing conventions in the file.
packages/theme/src/ee/pages/dashboard/settings/billing.vue (1)
12-23: LGTM!The billing page properly integrates the LocalLicenseAlert component and updates the plan management section with appropriate links and messaging.
packages/theme/src/ee/components/license/LocalLicenseAlert.vue (1)
3-3: The import statement is correct and resolves properly.The import
import { Alert } from "../../../components/ui/Alert"is valid. The path correctly resolves topackages/theme/src/components/ui/Alert/index.ts, which re-exports Alert as a named export. While Alert.vue itself uses a default export, the index.ts wrapper intentionally re-exports it as a named export for consumer use. This is a standard pattern and the named import syntax is appropriate here.Likely an incorrect or invalid review comment.
packages/theme/src/ee/pages/dashboard/settings/roles/Index.vue (1)
28-30: LGTM! Clean license gating implementation.The
LicenseRequiredwrapper appropriately gates the table view based on license status, providing a clear separation between licensed and unlicensed UI states.packages/theme/src/ee/components/UpgradeTooltip.vue (2)
23-27: Clarify the intent of the disabled trigger in production.The
:disabled="IS_PROD && !hasValidLicense"condition disables the tooltip trigger when in production without a valid license. This may prevent the tooltip from appearing on hover, which could be confusing since users won't understand why the feature is blocked. Verify this is the intended behavior.If the intent is to disable the wrapped button/element while still showing the tooltip, consider moving the disabled logic to the wrapped component itself rather than the trigger. Alternatively, if you want to block all interaction including tooltip display, document this behavior with a comment.
<TooltipTrigger + <!-- Disables interaction in production when license is invalid --> :disabled="IS_PROD && !hasValidLicense" >
37-43: Good environment-aware messaging!The conditional messaging for development vs. production provides appropriate context to users in different environments.
packages/theme/src/components/EmptyScreen.vue (1)
16-63: Clean and reusable empty state component.The component structure is well-designed with appropriate conditional rendering, responsive classes, and flexible slot support.
packages/theme/src/ee/components/LicenseRequired.vue (1)
28-39: Well-structured license gate UI.The empty screen fallback provides clear messaging and a call-to-action for users who encounter license restrictions in production.
packages/server/src/utils/logchimpConfig.ts (1)
129-132: License configuration structure looks good.The license-related configuration fields are properly integrated into both file-based and environment-based config loading, with appropriate fallbacks to
DEFAULT_LOGCHIMP_PILOT_URL.Also applies to: 186-190
packages/server/src/ee/services/license/checkLicense.ts (1)
35-35: machineSignature is properly configured—no action required.The verification confirms that
machineSignatureis correctly wired throughout the codebase:
- It's defined in the
Configinterface as an optional property- It's populated from either a config file or the
LOGCHIMP_MACHINE_SIGNATUREenvironment variable vialogchimpConfig.ts- The environment variable is properly declared in
environment.d.ts- It's correctly used in
checkLicense.tsline 35- Docker configurations reference the environment variable appropriately
The code is correct as-is.
| # To enable enterprise-only features, add LogChimp license key. | ||
| LOGCHIMP_LICENSE_KEY="" | ||
| # Signature token for the LogChimp License API (used for self-hosted) | ||
| # Along with license key, a signature token will be provide to you. |
There was a problem hiding this comment.
Fix grammatical error in comment.
Change "will be provide" to "will be provided".
-# Along with license key, a signature token will be provide to you.
+# Along with license key, a signature token will be provided to you.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Along with license key, a signature token will be provide to you. | |
| # Along with license key, a signature token will be provided to you. |
🤖 Prompt for AI Agents
In packages/server/.env.example around line 17, the comment has a grammatical
error: "will be provide" should be corrected to "will be provided"; update the
line to read "Along with license key, a signature token will be provided to
you." preserving the comment marker and punctuation.
| const DEFAULT_SERVER_PORT = 8000; | ||
| const DEFAULT_DATABASE_PORT = 5432; | ||
| const DEFAULT_MAIL_PORT = 465; | ||
| const DEFAULT_LOGCHIMP_PILOT_URL = "https://pilot.logchimp.codecarrot.net"; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Address code duplication of DEFAULT_LOGCHIMP_PILOT_URL.
The constant DEFAULT_LOGCHIMP_PILOT_URL is defined in both the server package (here) and the theme package (packages/theme/src/constants.ts). Consider moving this to a shared constants file in the @logchimp/types package or another shared location to maintain a single source of truth.
Based on learnings
🤖 Prompt for AI Agents
In packages/server/src/utils/logchimpConfig.ts around line 9, the constant
DEFAULT_LOGCHIMP_PILOT_URL is duplicated across this repo and
packages/theme/src/constants.ts; move this constant to a single shared location
(for example add it to @logchimp/types or a new shared constants module), update
both packages to import the constant from that shared module, and remove the
local duplicate definition here so both server and theme reference the same
source of truth.
| import Breadcrumbs from "../../../../components/Breadcrumbs.vue"; | ||
| import DashboardPageHeader from "../../../../components/dashboard/PageHeader.vue"; | ||
| import BreadcrumbItem from "../../../../components/ui/breadcrumbs/BreadcrumbItem.vue"; | ||
| import { DEFAULT_LOGCHIMP_PILOT_URL } from "../../../../constants.ts"; |
There was a problem hiding this comment.
Remove the .ts extension from the import statement.
Import statements in Vue/Vite projects should not include the .ts extension. This is non-standard and may cause issues with module resolution.
Apply this diff:
-import { DEFAULT_LOGCHIMP_PILOT_URL } from "../../../../constants.ts";
+import { DEFAULT_LOGCHIMP_PILOT_URL } from "../../../../constants";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { DEFAULT_LOGCHIMP_PILOT_URL } from "../../../../constants.ts"; | |
| import { DEFAULT_LOGCHIMP_PILOT_URL } from "../../../../constants"; |
🤖 Prompt for AI Agents
In packages/theme/src/ee/pages/dashboard/settings/billing.vue around line 34,
the import includes a .ts extension which is non-standard for Vue/Vite module
resolution; remove the ".ts" suffix from the import path so it imports from
"../../../../constants" instead, and save the file to ensure the project uses
the extensionless module import.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
CodeQL found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
packages/server/src/ee/routes/v1/posts.ts (1)
20-53: License gating is correctly applied, but address TypeScript suppressions.The middleware integration is consistent across all post-related routes. However, the multiple
@ts-expect-errordirectives (lines 27, 35, 42, 49) suppress type errors that should be resolved at the source.The typing issue likely stems from middleware signature mismatches with Express 5.x typed route handlers. Consider aligning the middleware signatures or using proper type guards instead of error suppressions.
packages/server/src/ee/routes/v1/boards.ts (1)
18-65: Comprehensive license gating across all board routes.The
licenseGuardmiddleware is consistently applied to all routes, including GET endpoints and the deprecatedcheck-nameroute (line 64), which correctly maintains uniform access control.However, the recurring
@ts-expect-errordirectives (lines 20, 26, 32, 39) suppress type errors that stem from middleware signature mismatches.Consider resolving the root cause of these type errors by:
- Ensuring middleware signatures match Express 5.x expectations
- Using type guards or proper middleware typing patterns
- Documenting why suppressions are necessary if they cannot be resolved
This pattern appears across multiple route files (roles, posts, boards), suggesting a systemic typing issue that should be addressed project-wide.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/server/src/controllers/settings/siteSettings.ts(2 hunks)packages/server/src/ee/middleware/licenseGuard.ts(1 hunks)packages/server/src/ee/routes/v1/boards.ts(1 hunks)packages/server/src/ee/routes/v1/posts.ts(1 hunks)packages/server/src/ee/routes/v1/roadmaps.ts(1 hunks)packages/server/src/ee/routes/v1/roles.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
packages/server/src/ee/routes/v1/roadmaps.ts (3)
packages/server/src/ee/middleware/licenseGuard.ts (1)
licenseGuard(7-24)packages/server/src/middlewares/auth/index.ts (1)
authRequired(5-5)packages/server/src/middlewares/roadmapExists.ts (1)
roadmapExists(18-66)
packages/server/src/ee/routes/v1/roles.ts (3)
packages/server/src/ee/middleware/licenseGuard.ts (1)
licenseGuard(7-24)packages/server/src/middlewares/auth/index.ts (1)
authRequired(5-5)packages/server/src/ee/middleware/roleExists.ts (1)
roleExists(8-40)
packages/server/src/ee/middleware/licenseGuard.ts (2)
packages/types/src/common.ts (1)
IApiErrorResponse(36-41)packages/server/src/ee/services/license/checkLicense.ts (1)
checkLicense(15-49)
packages/server/src/ee/routes/v1/posts.ts (3)
packages/types/src/posts/activity.ts (2)
IGetPostActivityRequestParam(24-26)IGetPostActivityRequestQuery(28-31)packages/server/src/ee/middleware/licenseGuard.ts (1)
licenseGuard(7-24)packages/server/src/middlewares/auth/index.ts (1)
authRequired(5-5)
packages/server/src/ee/routes/v1/boards.ts (3)
packages/server/src/ee/middleware/licenseGuard.ts (1)
licenseGuard(7-24)packages/server/src/ee/middleware/boardExists.ts (1)
boardExists(17-53)packages/server/src/middlewares/auth/index.ts (1)
authRequired(5-5)
packages/server/src/controllers/settings/siteSettings.ts (1)
packages/server/src/ee/services/license/checkLicense.ts (1)
checkLicense(15-49)
🪛 GitHub Actions: Lighthouse test
packages/server/src/ee/middleware/licenseGuard.ts
[error] 18-18: TS2339: Property 'license' does not exist on type '{ auth: { accessDenied: string; authorizationFailed: string; internalServerError: string; invalidAuthHeaderFormat: string; invalidAuthHeader: string; invalidToken: string; }; user: { userNotFound: string; userBlocked: string; incorrectPassword: string; userExists: string; }; }'.
🪛 GitHub Actions: Playwright Tests
packages/server/src/ee/middleware/licenseGuard.ts
[error] 18-18: TypeScript error TS2339: Property 'license' does not exist on type '{ auth: { accessDenied: string; authorizationFailed: string; internalServerError: string; invalidAuthHeaderFormat: string; invalidAuthHeader: string; invalidToken: string; }; user: { userNotFound: string; userBlocked: string; incorrectPassword: string; userExists: string; }; }'.
🪛 GitHub Actions: Test
packages/server/src/ee/middleware/licenseGuard.ts
[error] 18-18: TS2339: Property 'license' does not exist on type '{ auth: { accessDenied: string; authorizationFailed: string; internalServerError: string; invalidAuthHeaderFormat: string; invalidAuthHeader: string; invalidToken: string; }; user: { userNotFound: string; userBlocked: string; incorrectPassword: string; userExists: string; }; }'.
🪛 GitHub Check: CodeQL
packages/server/src/ee/routes/v1/roadmaps.ts
[failure] 13-13: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
This route handler performs authorization, but is not rate-limited.
[failure] 13-13: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.
[failure] 18-18: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
This route handler performs authorization, but is not rate-limited.
[failure] 19-19: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
[failure] 20-20: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
[failure] 22-22: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
This route handler performs authorization, but is not rate-limited.
[failure] 22-22: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.
[failure] 25-25: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
[failure] 26-26: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
[failure] 27-27: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
This route handler performs authorization, but is not rate-limited.
packages/server/src/ee/routes/v1/roles.ts
[failure] 19-19: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
This route handler performs authorization, but is not rate-limited.
[failure] 19-19: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
[failure] 29-29: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
This route handler performs authorization, but is not rate-limited.
[failure] 29-29: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
[failure] 31-31: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
This route handler performs authorization, but is not rate-limited.
[failure] 31-31: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
[failure] 31-31: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.
packages/server/src/ee/routes/v1/posts.ts
[failure] 29-29: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
packages/server/src/ee/routes/v1/boards.ts
[failure] 45-45: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
This route handler performs authorization, but is not rate-limited.
[failure] 45-45: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
[failure] 46-46: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
This route handler performs authorization, but is not rate-limited.
[failure] 46-46: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
[failure] 50-50: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
This route handler performs authorization, but is not rate-limited.
[failure] 51-51: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
[failure] 52-52: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.
[failure] 58-58: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
This route handler performs authorization, but is not rate-limited.
[failure] 59-59: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
[failure] 60-60: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
[failure] 64-64: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
This route handler performs authorization, but is not rate-limited.
[failure] 64-64: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
🔇 Additional comments (3)
packages/server/src/controllers/settings/siteSettings.ts (1)
23-29: LGTM! Clean integration of license validation.The license check is properly integrated into the site settings response, with appropriate error handling already in place via the existing try-catch block. The spread operator correctly merges the license status with existing settings.
packages/server/src/ee/routes/v1/roadmaps.ts (1)
13-30: LGTM! Consistent license gating across roadmap routes.The
licenseGuardmiddleware is correctly placed beforeauthRequiredin all routes, enabling fail-fast behavior when the license is invalid. The middleware ordering is appropriate for the access control flow.packages/server/src/ee/routes/v1/roles.ts (1)
19-51: @ts-expect-error suppressions are expected—not specific to licenseGuard.All middleware (
licenseGuard,authRequired,roleExists,userExists) follow the correct Express middleware signature. The@ts-expect-errorcomments are necessary due to a known TypeScript incompatibility with Express Router generics and async middleware—this pattern is consistent across the codebase (roles.ts, posts.ts, boards.ts). Routes without generics don't require these suppressions, confirming the issue is with the generic type parameters, not the middleware itself. No action required.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/server/src/errorResponse.json (1)
17-19: Align terminology between property name and error message.The property is named
verificationbut the message says "validation"—consider aligning these terms for consistency. Usingverificationin both places would match the property name."license": { - "verification": "LogChimp license validation failed" + "verification": "LogChimp license verification failed" }Verify that the licenseGuard middleware and any related license check logic consistently use either "validation" or "verification" throughout the codebase to ensure terminology alignment.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
packages/server/src/ee/middleware/licenseGuard.ts (1)
7-22: Wrap async middleware in try-catch for Express 4.x compatibility.This project uses Express 4.17.1, which does not automatically catch Promise rejections from async middleware. If
checkLicense()throws or rejects unexpectedly, the request will hang or crash.Apply this diff to add error handling:
export async function licenseGuard( _: Request, res: Response<IApiErrorResponse>, next: NextFunction, ) { + try { const result = await checkLicense(); if ("code" in result || result.status !== "active") { return res.status(403).send({ message: error.middleware.license.verification, code: "LICENSE_VALIDATION_FAILED", }); } next(); + } catch (err) { + return res.status(500).send({ + message: error.middleware.license.verification, + code: "LICENSE_VALIDATION_ERROR", + }); + } }packages/server/src/ee/services/license/checkLicense.ts (1)
179-184: Add Content-Type header to fetch request.The fetch request is missing a
Content-Type: application/jsonheader, which may cause the license server to reject or misinterpret the request body.Apply this diff:
const response = await fetch(`${config.licensePilotUrl}/v1/license/check`, { method: "POST", mode: "cors", + headers: { + "Content-Type": "application/json", + }, body: JSON.stringify(requestBody), signal: controller.signal, });packages/types/src/license.ts (1)
12-12: Changetimestamp: Datetotimestamp: stringin ICheckLicenseResponseBodyThe interface incorrectly declares
timestampasDate, butresponse.json()deserializes the HTTP response and returns the timestamp as a string. JSON parsing never converts timestamp strings to Date objects automatically. This type mismatch will cause runtime errors if code attempts to use Date methods on the timestamp field.The parallel interface
ICheckLicenseDecryptedPayloadcorrectly usesserver_time: string, which should be consistent.export interface ICheckLicenseResponseBody { encrypted_payload: string; - timestamp: Date; + timestamp: string; }
🧹 Nitpick comments (3)
packages/types/src/license.ts (2)
5-5: Remove redundant type union.The type
string | "unknown"is redundant since"unknown"is already a string literal that satisfiesstring.Apply this diff:
- deploymentProvider: string | "unknown"; + deploymentProvider: string;
20-20: Remove commented code or implement the feature.The commented
featuresfield suggests incomplete implementation. Either implement feature-based licensing or remove the comment to avoid dead code.packages/server/src/ee/services/license/checkLicense.ts (1)
55-59: Consider returning early when license signature is missing.The code logs a warning for missing
licenseSignaturebut continues execution, unlike the missinglicenseKeycase which returns early. SincedecryptPayload()throws an error when signature is missing (line 206-208), this will eventually fail anyway. Consider returning early for consistency and clearer error flow.Apply this diff:
if (!config.licenseSignature) { logger.warn( "License signature is not configured. Check your environment variables or 'logchimp.config.json' file.", ); + return EMPTY_LICENSE_RESPONSE; }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/server/src/constants.ts(1 hunks)packages/server/src/ee/middleware/licenseGuard.ts(1 hunks)packages/server/src/ee/services/license/checkLicense.ts(1 hunks)packages/types/src/license.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/server/src/constants.ts
🧰 Additional context used
🧬 Code graph analysis (2)
packages/server/src/ee/middleware/licenseGuard.ts (2)
packages/types/src/common.ts (1)
IApiErrorResponse(36-41)packages/server/src/ee/services/license/checkLicense.ts (1)
checkLicense(45-78)
packages/server/src/ee/services/license/checkLicense.ts (4)
packages/server/src/utils/logchimpConfig.ts (1)
configManager(247-247)packages/types/src/license.ts (3)
ICheckLicenseDecryptedPayload(15-21)ICheckLicenseResponseBody(10-13)ICheckLicenseRequestBody(1-8)packages/types/src/common.ts (1)
IApiErrorResponse(36-41)packages/server/src/constants.ts (1)
LOGCHIMP_LICENSE_CACHE_TTL_IN_SEC(10-10)
🔇 Additional comments (3)
packages/server/src/ee/services/license/checkLicense.ts (3)
103-108: LGTM with minor type safety note.The type guard
"code" in datacorrectly identifiesIApiErrorResponse. The fallback toEMPTY_LICENSE_RESPONSEensures callers receive a consistent structure. Caching error responses prevents repeated failed requests.
229-239: LGTM!The deployment provider detection is clean and covers common platforms with a sensible fallback.
80-101: LGTM!The dual-layer caching strategy (memory + external cache) with error handling is well-designed. Cache reads gracefully degrade on failure, and the external cache updates are wrapped in try-catch to prevent cache failures from breaking the license check flow.
| message: "failed to decrypt license payload", | ||
| error: e, | ||
| }); | ||
| console.log(e); |
There was a problem hiding this comment.
Remove debug console.log statement.
This debug statement should be removed before merging to production, as error logging is already handled on line 141-144.
Apply this diff:
logger.error({
message: "failed to decrypt license payload",
error: e,
});
- console.log(e);
return EMPTY_LICENSE_RESPONSE;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| console.log(e); | |
| logger.error({ | |
| message: "failed to decrypt license payload", | |
| error: e, | |
| }); | |
| return EMPTY_LICENSE_RESPONSE; |
🤖 Prompt for AI Agents
In packages/server/src/ee/services/license/checkLicense.ts around line 145,
remove the leftover debug console.log(e); statement and ensure no other direct
console logging remains; rely on the existing error handling/logging already
implemented on lines 141–144 (or replace the console.log with
processLogger.error(...) only if additional context is required).
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/server/src/ee/routes/v1/license.ts(1 hunks)packages/server/src/routes/v1/index.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/server/src/ee/routes/v1/license.ts
mittalyashu
force-pushed
the
feat/license-validation
branch
from
14e4458 to
4078c74
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (8)
packages/theme/src/ee/pages/dashboard/settings/billing.vue (1)
34-34: Remove the.tsextension from the import statement.Import statements in Vue/Vite projects should not include the
.tsextension, as this is non-standard and may cause module resolution issues.Apply this diff:
-import { DEFAULT_LOGCHIMP_PILOT_URL } from "../../../../constants.ts"; +import { DEFAULT_LOGCHIMP_PILOT_URL } from "../../../../constants";packages/server/.env.example (1)
18-18: Fix grammatical error in comment.Change "This ensure" to "This ensures".
Apply this diff:
-# This ensure you and only you can communicate with the LogChimp License API for your license key. +# This ensures you and only you can communicate with the LogChimp License API for your license key.packages/server/src/ee/routes/v1/license.ts (1)
6-6: Critical: Import path cannot be resolved.The import path
../../controllers/v1/checkLicensedoes not exist, causing TypeScript compilation failures. This issue has already been flagged in previous reviews and pipeline failures.packages/server/src/ee/middleware/licenseGuard.ts (1)
7-22: Add try-catch error handling for Express 4.x async middleware.Express 4.17.1 does not automatically catch Promise rejections from async middleware. If
checkLicense()throws or rejects, the request will hang or crash. Wrap the middleware body in try-catch and callnext(error)on errors.Apply this diff to add error handling:
export async function licenseGuard( _: Request, res: Response<IApiErrorResponse>, next: NextFunction, ) { + try { const result = await checkLicense(); if ("code" in result || result.status !== "active") { return res.status(403).send({ message: error.middleware.license.verification, code: "LICENSE_VALIDATION_FAILED", }); } next(); + } catch (error) { + next(error); + } }Based on learnings about Express 4.x async middleware.
packages/server/src/ee/services/license/checkLicense.ts (4)
43-43: Fix type mismatch in pendingLicenseCheck.The variable is typed as
Promise<ICheckLicenseDecryptedPayload> | null, butcheckLicense()returnsPromise<ICheckLicenseDecryptedPayload | IApiErrorResponse>. This creates a type inconsistency at line 67 wherependingLicenseCheckis returned.Apply this diff:
-let pendingLicenseCheck: Promise<ICheckLicenseDecryptedPayload> | null = null; +let pendingLicenseCheck: Promise<ICheckLicenseDecryptedPayload | IApiErrorResponse> | null = null;
145-145: Remove debug console.log statement.This debug statement should be removed before merging to production, as error logging is already handled via
logger.erroron lines 141-144.Apply this diff:
logger.error({ message: "failed to decrypt license payload", error: e, }); - console.log(e); return EMPTY_LICENSE_RESPONSE;
210-224: Implement JWT expiration validation.The TODO comment on line 215 indicates JWT expiration checking is not implemented. Without this validation, expired license tokens will be accepted, which is a compliance risk.
The
jwt.verify()method automatically validates theexpclaim if present in the JWT and throwsTokenExpiredErrorif expired. Wrap the verification in try-catch to handle expiration explicitly:+ try { const decoded = jwt.verify( encryptedPayload, config.licenseSignature, ) as ICheckLicenseDecryptedPayload; - // todo: check is jwt is expired if (decoded.license_key !== config.licenseKey) { try { await clearCache(); } catch (_) {} throw new Error( "License key mismatch: payload does not match requesting license", ); } return decoded; + } catch (error) { + if (error.name === "TokenExpiredError") { + await clearCache().catch(() => {}); + throw new Error("License has expired"); + } + throw error; + }Ensure the license server includes an
expclaim in the JWT payload.
179-184: Add Content-Type header to license server request.The fetch request is missing the
Content-Type: application/jsonheader, which may cause the license server to reject or misinterpret the request body.Apply this diff:
const response = await fetch(`${config.licensePilotUrl}/v1/license/check`, { method: "POST", mode: "cors", + headers: { + "Content-Type": "application/json", + }, body: JSON.stringify(requestBody), signal: controller.signal, });
🧹 Nitpick comments (2)
packages/theme/src/ee/pages/dashboard/settings/billing.vue (1)
41-43: Component name doesn't match filename.The component name "DashboardSettings" doesn't match the file name "billing.vue". Consider using a more descriptive name like "DashboardBillingSettings" for consistency and easier debugging in Vue DevTools.
Apply this diff:
defineOptions({ - name: "DashboardSettings", + name: "DashboardBillingSettings", });packages/server/src/ee/routes/v1/boards.ts (1)
20-21: Add explanatory comment documenting the async middleware type limitation.The
@ts-expect-errorsuppressions are justified and necessary—they work around a limitation in Express 4.17.1's TypeScript definitions when using async middleware in generic route handlers (e.g.,router.get<T>()). Async middleware returnsPromise<void>, which doesn't match the strict type expectations. This pattern is consistent across the codebase for all async middleware.Rather than fixing an underlying defect, consider adding a comment at the top of the file explaining this limitation for future maintainers:
// Note: @ts-expect-error is used for async middleware (licenseGuard, boardExists) // because Express 4.17.1's types don't properly support async middleware in // generic route handlers. The middleware functions correctly at runtime.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (31)
docker/community/docker-compose.yml(1 hunks)packages/server/.env.example(1 hunks)packages/server/src/constants.ts(1 hunks)packages/server/src/controllers/settings/siteSettings.ts(2 hunks)packages/server/src/ee/middleware/licenseGuard.ts(1 hunks)packages/server/src/ee/routes/v1/boards.ts(1 hunks)packages/server/src/ee/routes/v1/license.ts(1 hunks)packages/server/src/ee/routes/v1/posts.ts(1 hunks)packages/server/src/ee/routes/v1/roadmaps.ts(1 hunks)packages/server/src/ee/routes/v1/roles.ts(2 hunks)packages/server/src/ee/services/license/checkLicense.ts(1 hunks)packages/server/src/environment.d.ts(1 hunks)packages/server/src/errorResponse.json(1 hunks)packages/server/src/routes/v1/index.ts(2 hunks)packages/server/src/utils/logchimpConfig.ts(7 hunks)packages/theme/.env.example(1 hunks)packages/theme/inject-app-env.sh(1 hunks)packages/theme/src/components/EmptyScreen.vue(1 hunks)packages/theme/src/constants.ts(2 hunks)packages/theme/src/ee/components/LicenseRequired.vue(1 hunks)packages/theme/src/ee/components/UpgradeTooltip.vue(1 hunks)packages/theme/src/ee/components/license/LocalLicenseAlert.vue(1 hunks)packages/theme/src/ee/pages/dashboard/boards/Index.vue(2 hunks)packages/theme/src/ee/pages/dashboard/roadmaps/Index.vue(2 hunks)packages/theme/src/ee/pages/dashboard/settings/billing.vue(2 hunks)packages/theme/src/ee/pages/dashboard/settings/roles/Index.vue(2 hunks)packages/theme/src/pages/dashboard/posts/Index.vue(3 hunks)packages/theme/src/store/settings.ts(1 hunks)packages/types/src/index.ts(1 hunks)packages/types/src/license.ts(1 hunks)packages/types/src/site/settings.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (15)
- packages/types/src/index.ts
- packages/theme/src/ee/pages/dashboard/boards/Index.vue
- packages/server/src/constants.ts
- packages/theme/src/constants.ts
- packages/theme/src/ee/components/LicenseRequired.vue
- packages/types/src/license.ts
- packages/server/src/routes/v1/index.ts
- packages/server/src/errorResponse.json
- packages/theme/src/ee/components/UpgradeTooltip.vue
- packages/theme/src/store/settings.ts
- packages/theme/src/pages/dashboard/posts/Index.vue
- packages/theme/src/components/EmptyScreen.vue
- docker/community/docker-compose.yml
- packages/theme/src/ee/components/license/LocalLicenseAlert.vue
- packages/types/src/site/settings.ts
🧰 Additional context used
🧬 Code graph analysis (8)
packages/server/src/controllers/settings/siteSettings.ts (1)
packages/server/src/ee/services/license/checkLicense.ts (1)
checkLicense(45-78)
packages/server/src/ee/services/license/checkLicense.ts (4)
packages/server/src/utils/logchimpConfig.ts (1)
configManager(247-247)packages/types/src/license.ts (3)
ICheckLicenseDecryptedPayload(15-21)ICheckLicenseResponseBody(10-13)ICheckLicenseRequestBody(1-8)packages/types/src/common.ts (1)
IApiErrorResponse(36-41)packages/server/src/constants.ts (1)
LOGCHIMP_LICENSE_CACHE_TTL_IN_SEC(10-10)
packages/server/src/utils/logchimpConfig.ts (1)
packages/theme/src/constants.ts (1)
DEFAULT_LOGCHIMP_PILOT_URL(15-16)
packages/server/src/ee/middleware/licenseGuard.ts (2)
packages/types/src/common.ts (1)
IApiErrorResponse(36-41)packages/server/src/ee/services/license/checkLicense.ts (1)
checkLicense(45-78)
packages/server/src/ee/routes/v1/roles.ts (3)
packages/server/src/ee/middleware/licenseGuard.ts (1)
licenseGuard(7-22)packages/server/src/middlewares/auth/index.ts (1)
authRequired(5-5)packages/server/src/ee/middleware/roleExists.ts (1)
roleExists(8-40)
packages/server/src/ee/routes/v1/boards.ts (4)
packages/types/src/board.ts (4)
TFilterBoardRequestQuery(30-30)IGetBoardsRequestQuery(20-24)IGetBoardByUrlRequestParams(36-38)ISearchBoardRequestParams(90-92)packages/server/src/ee/middleware/licenseGuard.ts (1)
licenseGuard(7-22)packages/server/src/ee/middleware/boardExists.ts (1)
boardExists(17-53)packages/server/src/middlewares/auth/index.ts (1)
authRequired(5-5)
packages/server/src/ee/routes/v1/posts.ts (3)
packages/types/src/posts/activity.ts (5)
IGetPostActivityRequestParam(24-26)IGetPostActivityRequestQuery(28-31)TCreatePostCommentRequestParam(37-37)IUpdatePostCommentRequestParam(49-52)TDeletePostCommentRequestParam(64-64)packages/server/src/ee/middleware/licenseGuard.ts (1)
licenseGuard(7-22)packages/server/src/middlewares/auth/index.ts (1)
authRequired(5-5)
packages/server/src/ee/routes/v1/roadmaps.ts (3)
packages/server/src/ee/middleware/licenseGuard.ts (1)
licenseGuard(7-22)packages/server/src/middlewares/auth/index.ts (1)
authRequired(5-5)packages/server/src/middlewares/roadmapExists.ts (1)
roadmapExists(18-66)
🪛 dotenv-linter (4.0.0)
packages/server/.env.example
[warning] 15-15: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 19-19: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 20-20: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 20-20: [UnorderedKey] The LOGCHIMP_PILOT_URL key should go before the LOGCHIMP_SIGNATURE_TOKEN key
(UnorderedKey)
packages/theme/.env.example
[warning] 2-2: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
🪛 GitHub Actions: Lighthouse test
packages/server/src/ee/routes/v1/license.ts
[error] 6-6: TS2307: Cannot find module '../../controllers/v1/checkLicense' or its corresponding type declarations.
🪛 GitHub Actions: Playwright Tests
packages/server/src/ee/routes/v1/license.ts
[error] 6-6: Cannot find module '../../controllers/v1/checkLicense' or its corresponding type declarations.
🪛 GitHub Actions: Test
packages/server/src/ee/routes/v1/license.ts
[error] 6-6: Cannot find module '../../controllers/v1/checkLicense' or its corresponding type declarations.
🪛 GitHub Check: CodeQL
packages/server/src/ee/routes/v1/roles.ts
[failure] 19-19: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
This route handler performs authorization, but is not rate-limited.
[failure] 19-19: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
[failure] 29-29: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
This route handler performs authorization, but is not rate-limited.
[failure] 29-29: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
[failure] 31-31: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
This route handler performs authorization, but is not rate-limited.
[failure] 31-31: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
[failure] 31-31: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.
packages/server/src/ee/routes/v1/boards.ts
[failure] 45-45: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
This route handler performs authorization, but is not rate-limited.
[failure] 45-45: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
[failure] 46-46: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
This route handler performs authorization, but is not rate-limited.
[failure] 46-46: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
[failure] 50-50: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
This route handler performs authorization, but is not rate-limited.
[failure] 51-51: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
[failure] 52-52: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.
[failure] 58-58: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
This route handler performs authorization, but is not rate-limited.
[failure] 59-59: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
[failure] 60-60: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
[failure] 64-64: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
This route handler performs authorization, but is not rate-limited.
[failure] 64-64: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
packages/server/src/ee/routes/v1/posts.ts
[failure] 29-29: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
packages/server/src/ee/routes/v1/roadmaps.ts
[failure] 13-13: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
This route handler performs authorization, but is not rate-limited.
[failure] 13-13: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.
[failure] 18-18: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
This route handler performs authorization, but is not rate-limited.
[failure] 19-19: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
[failure] 20-20: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
[failure] 22-22: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
This route handler performs authorization, but is not rate-limited.
[failure] 22-22: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.
[failure] 25-25: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
[failure] 26-26: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
[failure] 27-27: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
This route handler performs authorization, but is not rate-limited.
🔇 Additional comments (16)
packages/theme/.env.example (1)
2-2: Environment variable addition is well-integrated.The new
VITE_WEBAPP_URLvariable follows the established formatting and placement conventions. The static analysis warning about quote characters is a false positive—all environment values in the file consistently use double quotes.packages/theme/inject-app-env.sh (1)
8-8: Environment injection properly extended.The new
VITE_WEBAPP_URLvariable is correctly added to thewindow.__APP_ENV__object, using the same substitution pattern as existing variables. The shell variable expansion syntax is correct and consistent.packages/theme/src/ee/pages/dashboard/settings/billing.vue (2)
12-12: LGTM!The LocalLicenseAlert component integration looks good. The component is properly imported and used to replace the previous warning alert block.
Also applies to: 35-35
16-16: LGTM!The section title change from "Active Plan" to "Manage Plan" appropriately reflects the section's purpose of providing a link to external billing management.
packages/theme/src/ee/pages/dashboard/roadmaps/Index.vue (2)
30-32: LGTM!The
LicenseRequiredwrapper provides appropriate content-level gating for the roadmaps view. This complements the informational UI on the button (tooltip + crown icon) with actual enforcement.
46-46: LGTM!The imports and store initialization follow Vue 3 composition API best practices correctly.
Also applies to: 54-55, 59-59
packages/theme/src/ee/pages/dashboard/settings/roles/Index.vue (3)
28-30: LGTM - Consistent license gating pattern.The
LicenseRequiredwrapper appropriately gates the roles table view based on license validity, consistent with the pattern applied across other dashboard pages.
45-45: ****The settings store properly initializes
hasValidLicensewith a default value offalse. The reactive state is fully defined at store creation (line 16 inpackages/theme/src/store/settings.ts), ensuring that accessingsettingsStore.settings.hasValidLicensein the template will never result in undefined behavior. No changes are needed.Likely an incorrect or invalid review comment.
9-24: Let me verify thelicenseGuardmiddleware implementation to confirm it properly validates licenses:Backend license enforcement confirmed via
licenseGuardmiddleware.The script output clearly shows that the POST
/rolesendpoint has license validation applied at the middleware level (router.post("/roles", licenseGuard, authRequired, roles.create);). This means the backend will enforce license checks and reject requests from unlicensed users.However, from a UX perspective, the suggestion to disable the button frontend-side is still valid—the current implementation allows users to click a button that the backend will ultimately reject, which creates unnecessary confusion. Disabling the button when the license is invalid would provide clearer, immediate feedback and prevent failed requests.
Optional enhancement: Add
!settingsStore.settings.hasValidLicenseto thecreateRoleButtonDisabledcomputed property to disable the button frontend-side and improve UX clarity.packages/server/src/environment.d.ts (1)
16-19: LGTM!The new environment variable declarations for license key and signature token follow the existing pattern and are properly typed as optional strings.
packages/server/src/ee/routes/v1/posts.ts (1)
17-53: LGTM! License guard consistently applied.The
licenseGuardmiddleware is correctly integrated into all post activity and comment routes, ensuring enterprise features are properly gated.packages/server/src/ee/routes/v1/roadmaps.ts (1)
11-30: LGTM! License guard correctly applied to roadmap routes.The
licenseGuardmiddleware is properly integrated before authentication and other validation middleware on all roadmap mutation routes (create, update, sort, delete).packages/server/src/ee/routes/v1/roles.ts (1)
17-51: LGTM! License guard consistently applied to role routes.The
licenseGuardmiddleware is correctly integrated across all role-related routes, ensuring enterprise role management features are properly protected by license validation.packages/server/src/ee/routes/v1/boards.ts (2)
45-46: LGTM - License guard integration is consistent.The
licenseGuardmiddleware is correctly placed beforeauthRequiredandboardExistson write operations (POST/PATCH/DELETE), ensuring license validation occurs before authorization and resource checks. This follows the principle of failing fast on license issues.Also applies to: 47-53, 55-61, 64-64
45-64: Note: Rate limiting warnings are pre-existing.Static analysis flagged missing rate limiting on these routes. However, this is a pre-existing concern not introduced by this PR — the routes already performed authorization and database access before
licenseGuardwas added. Rate limiting should be addressed separately.packages/server/src/ee/services/license/checkLicense.ts (1)
80-149: LGTM - Robust dual-layer caching and error handling.The implementation includes:
- In-memory cache with TTL for fast access
- Optional external cache (Valkey) with proper error handling
- Concurrency coalescing via
pendingLicenseCheckto prevent request storms- Comprehensive error logging without crashing the flow
- Cache invalidation on license key mismatch
The architecture is well-designed for production use once the flagged issues are addressed.
| // private getEnv = <K extends keyof NodeJS.ProcessEnv>(key: K, fallback?: NodeJS.ProcessEnv[K]): NodeJS.ProcessEnv[K] => { | ||
| // const value = process.env[key] as NodeJS.ProcessEnv[K] | undefined; | ||
| // | ||
| // if (value === undefined) { | ||
| // // handle fallback falsy cases that should still be used as value | ||
| // if (fallback === false || fallback === "" || fallback === 0) { | ||
| // return fallback; | ||
| // } | ||
| // if (fallback) { | ||
| // return fallback; | ||
| // } | ||
| // throw new Error(`Missing environment variable: ${key}.`); | ||
| // } | ||
| // | ||
| // return value; | ||
| // }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove commented-out code.
The commented-out getEnv helper method (lines 229-244) should be removed. If this functionality is needed in the future, it can be retrieved from version control.
Apply this diff:
-
- // private getEnv = <K extends keyof NodeJS.ProcessEnv>(key: K, fallback?: NodeJS.ProcessEnv[K]): NodeJS.ProcessEnv[K] => {
- // const value = process.env[key] as NodeJS.ProcessEnv[K] | undefined;
- //
- // if (value === undefined) {
- // // handle fallback falsy cases that should still be used as value
- // if (fallback === false || fallback === "" || fallback === 0) {
- // return fallback;
- // }
- // if (fallback) {
- // return fallback;
- // }
- // throw new Error(`Missing environment variable: ${key}.`);
- // }
- //
- // return value;
- // };
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // private getEnv = <K extends keyof NodeJS.ProcessEnv>(key: K, fallback?: NodeJS.ProcessEnv[K]): NodeJS.ProcessEnv[K] => { | |
| // const value = process.env[key] as NodeJS.ProcessEnv[K] | undefined; | |
| // | |
| // if (value === undefined) { | |
| // // handle fallback falsy cases that should still be used as value | |
| // if (fallback === false || fallback === "" || fallback === 0) { | |
| // return fallback; | |
| // } | |
| // if (fallback) { | |
| // return fallback; | |
| // } | |
| // throw new Error(`Missing environment variable: ${key}.`); | |
| // } | |
| // | |
| // return value; | |
| // }; | |
| } |
🤖 Prompt for AI Agents
In packages/server/src/utils/logchimpConfig.ts around lines 229 to 244, remove
the commented-out getEnv helper method block (lines 229-244) entirely; delete
the commented code so the file no longer contains dead/commented-out
implementation—if needed later it can be retrieved from version control.
|
Lighthouse report
|
mittalyashu
force-pushed
the
feat/license-validation
branch
from
c75bf44 to
163772f
Compare
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/theme/src/components/dashboard/posts/PostEditor/index.vue (1)
101-133: FixhasValidLicensereactivity in PostEditor.
const { hasValidLicense } = useSettingsEEStore();captures a non-reactive boolean. Once the license check flips to true, the crowns/tooltips stay visible and the UX remains blocked. Keep the store instance and convert to refs.-import { computed, ref, reactive } from "vue"; +import { computed, ref, reactive } from "vue"; +import { storeToRefs } from "pinia"; @@ -import { useSettingsEEStore } from "../../../../ee/store/settings"; +import { useSettingsEEStore } from "../../../../ee/store/settings"; @@ -const { hasValidLicense } = useSettingsEEStore(); +const settingsEEStore = useSettingsEEStore(); +const { hasValidLicense } = storeToRefs(settingsEEStore);
♻️ Duplicate comments (6)
packages/server/src/utils/logchimpConfig.ts (2)
229-244: Remove commented-out code.The commented
getEnvhelper method was flagged for removal in a previous review but still remains. Dead code should be removed.
10-10: Duplicate constant definition.This constant is duplicated between server and theme packages, as noted in the previous review. The duplication still exists.
Based on learnings
packages/types/src/license.ts (1)
10-13: Timestamp type still incorrect despite previous review.The past review flagged that
timestamp: Dateshould betimestamp: stringsince JSON parsing returns strings, not Date objects. However, this issue persists in the current code.Apply this diff:
export interface ICheckLicenseResponseBody { encrypted_payload: string; - timestamp: Date; + timestamp: string; }packages/server/src/ee/services/license/checkLicense.ts (3)
43-43: Type mismatch in pendingLicenseCheck.The variable type doesn't match the actual return type from
checkLicense(), as noted in the previous review. This issue persists.Apply this diff:
-let pendingLicenseCheck: Promise<ICheckLicenseDecryptedPayload> | null = null; +let pendingLicenseCheck: Promise<ICheckLicenseDecryptedPayload | IApiErrorResponse> | null = null;
145-145: Remove debug console.log statement.This debug statement was flagged in a previous review and should be removed.
Apply this diff:
logger.error({ message: "failed to decrypt license payload", error: e, }); - console.log(e); return EMPTY_LICENSE_RESPONSE;
203-227: JWT expiration validation is incomplete.The TODO comment at line 215 indicates JWT expiration checking is not fully implemented, as noted in the previous review. The
jwt.verify()function will automatically validate theexpclaim if present in the JWT, but you should verify this is included by the license server or implement custom expiration validation.Ensure the license server includes an
expclaim in the JWT, or add explicit expiration validation:const decoded = jwt.verify( encryptedPayload, config.licenseSignature, ) as ICheckLicenseDecryptedPayload; - // todo: check is jwt is expired + // jwt.verify automatically validates 'exp' if present + // Ensure license server includes 'exp' claim in JWT payload if (decoded.license_key !== config.licenseKey) {
🧹 Nitpick comments (4)
packages/theme/.env.example (1)
1-5: Optional: Document variable purpose.Adding an inline comment above this new variable would help developers understand its role (e.g., used for license validation). This is a nice-to-have to improve clarity for future maintainers, but not essential.
Example:
VITE_API_URL="http://localhost:8000" +# Webapp base URL for license verification and external references VITE_WEBAPP_URL="http://localhost:3000"packages/server/src/ee/controllers/v1/checkLicense.ts (2)
9-14: Consider using a status constant.The authorization logic correctly handles both error responses and non-active statuses. However, the hardcoded
"active"string could be replaced with a constant for better maintainability.Optional improvement:
+const LICENSE_STATUS_ACTIVE = "active"; + export async function checkLicenseController(req, res, next) { try { const result = await checkLicenseService(); - if ("code" in result || result.status !== "active") { + if ("code" in result || result.status !== LICENSE_STATUS_ACTIVE) { return res.status(403).send({
19-28: Improve error logging consistency.The error logging lacks a
codeproperty for categorization, which is inconsistent with the pattern used elsewhere in the codebase (e.g.,packages/server/src/database/config.tslines 44-49).Apply this diff:
} catch (e) { logger.error({ + code: "LICENSE_CHECK_CONTROLLER", message: e, });packages/types/src/license.ts (1)
15-21: Remove commented field and consider status literal type.The commented
featuresfield should either be implemented or removed. Additionally, sincestatusis checked against the literal"active"in the controller, consider using a string literal union for type safety.Apply this diff:
export interface ICheckLicenseDecryptedPayload { response_nonce: string; license_key: string; server_time: string; - status: string; - // features: string[]; + status: "active" | "inactive" | "expired" | "suspended" | ""; }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (38)
docker/community/docker-compose.yml(1 hunks)packages/server/.env.example(1 hunks)packages/server/src/constants.ts(1 hunks)packages/server/src/controllers/settings/siteSettings.ts(2 hunks)packages/server/src/ee/controllers/v1/checkLicense.ts(1 hunks)packages/server/src/ee/middleware/licenseGuard.ts(1 hunks)packages/server/src/ee/routes/v1/boards.ts(1 hunks)packages/server/src/ee/routes/v1/license.ts(1 hunks)packages/server/src/ee/routes/v1/posts.ts(1 hunks)packages/server/src/ee/routes/v1/roadmaps.ts(1 hunks)packages/server/src/ee/routes/v1/roles.ts(2 hunks)packages/server/src/ee/services/license/checkLicense.ts(1 hunks)packages/server/src/environment.d.ts(1 hunks)packages/server/src/errorResponse.json(1 hunks)packages/server/src/routes/v1/index.ts(2 hunks)packages/server/src/utils/logchimpConfig.ts(7 hunks)packages/theme/.env.example(1 hunks)packages/theme/inject-app-env.sh(1 hunks)packages/theme/src/components/EmptyScreen.vue(1 hunks)packages/theme/src/components/dashboard/posts/PostEditor/index.vue(3 hunks)packages/theme/src/constants.ts(2 hunks)packages/theme/src/ee/components/LicenseRequired.vue(1 hunks)packages/theme/src/ee/components/UpgradeTooltip.vue(1 hunks)packages/theme/src/ee/components/dashboard/boards/SearchBoardDropdown/Dropdown.vue(2 hunks)packages/theme/src/ee/components/dashboard/roadmap/SearchRoadmapDropdown/Dropdown.vue(2 hunks)packages/theme/src/ee/components/icons/LicenseCrown.vue(1 hunks)packages/theme/src/ee/components/license/LocalLicenseAlert.vue(1 hunks)packages/theme/src/ee/modules/settings.ts(1 hunks)packages/theme/src/ee/pages/dashboard/boards/Index.vue(1 hunks)packages/theme/src/ee/pages/dashboard/roadmaps/Index.vue(1 hunks)packages/theme/src/ee/pages/dashboard/settings/billing.vue(2 hunks)packages/theme/src/ee/pages/dashboard/settings/roles/Index.vue(1 hunks)packages/theme/src/ee/store/settings.ts(1 hunks)packages/theme/src/pages/dashboard/posts/Index.vue(1 hunks)packages/theme/src/store/settings.ts(1 hunks)packages/types/src/index.ts(1 hunks)packages/types/src/license.ts(1 hunks)packages/types/src/site/settings.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (18)
- packages/theme/src/constants.ts
- packages/server/src/environment.d.ts
- packages/types/src/site/settings.ts
- packages/theme/src/ee/components/dashboard/roadmap/SearchRoadmapDropdown/Dropdown.vue
- docker/community/docker-compose.yml
- packages/theme/src/ee/modules/settings.ts
- packages/theme/src/ee/pages/dashboard/settings/billing.vue
- packages/theme/src/ee/components/icons/LicenseCrown.vue
- packages/theme/src/ee/store/settings.ts
- packages/theme/inject-app-env.sh
- packages/theme/src/pages/dashboard/posts/Index.vue
- packages/types/src/index.ts
- packages/theme/src/ee/pages/dashboard/roadmaps/Index.vue
- packages/server/src/constants.ts
- packages/theme/src/components/EmptyScreen.vue
- packages/server/src/routes/v1/index.ts
- packages/theme/src/ee/components/license/LocalLicenseAlert.vue
- packages/server/src/controllers/settings/siteSettings.ts
🧰 Additional context used
🧬 Code graph analysis (8)
packages/server/src/ee/controllers/v1/checkLicense.ts (1)
packages/server/src/database/config.ts (1)
error(45-50)
packages/server/src/ee/middleware/licenseGuard.ts (2)
packages/types/src/common.ts (1)
IApiErrorResponse(36-41)packages/server/src/ee/services/license/checkLicense.ts (1)
checkLicense(45-78)
packages/server/src/utils/logchimpConfig.ts (1)
packages/theme/src/constants.ts (1)
DEFAULT_LOGCHIMP_PILOT_URL(15-16)
packages/server/src/ee/services/license/checkLicense.ts (4)
packages/server/src/utils/logchimpConfig.ts (1)
configManager(247-247)packages/types/src/license.ts (3)
ICheckLicenseDecryptedPayload(15-21)ICheckLicenseResponseBody(10-13)ICheckLicenseRequestBody(1-8)packages/types/src/common.ts (1)
IApiErrorResponse(36-41)packages/server/src/constants.ts (1)
LOGCHIMP_LICENSE_CACHE_TTL_IN_SEC(10-10)
packages/server/src/ee/routes/v1/boards.ts (4)
packages/types/src/board.ts (4)
TFilterBoardRequestQuery(30-30)IGetBoardsRequestQuery(20-24)IGetBoardByUrlRequestParams(36-38)ISearchBoardRequestParams(90-92)packages/server/src/ee/middleware/licenseGuard.ts (1)
licenseGuard(8-34)packages/server/src/ee/middleware/boardExists.ts (1)
boardExists(17-53)packages/server/src/middlewares/auth/index.ts (1)
authRequired(5-5)
packages/server/src/ee/routes/v1/posts.ts (3)
packages/types/src/posts/activity.ts (5)
IGetPostActivityRequestParam(24-26)IGetPostActivityRequestQuery(28-31)TCreatePostCommentRequestParam(37-37)IUpdatePostCommentRequestParam(49-52)TDeletePostCommentRequestParam(64-64)packages/server/src/ee/middleware/licenseGuard.ts (1)
licenseGuard(8-34)packages/server/src/middlewares/auth/index.ts (1)
authRequired(5-5)
packages/server/src/ee/routes/v1/roadmaps.ts (3)
packages/server/src/ee/middleware/licenseGuard.ts (1)
licenseGuard(8-34)packages/server/src/middlewares/auth/index.ts (1)
authRequired(5-5)packages/server/src/middlewares/roadmapExists.ts (1)
roadmapExists(18-66)
packages/server/src/ee/routes/v1/roles.ts (3)
packages/server/src/ee/middleware/licenseGuard.ts (1)
licenseGuard(8-34)packages/server/src/middlewares/auth/index.ts (1)
authRequired(5-5)packages/server/src/ee/middleware/roleExists.ts (1)
roleExists(8-40)
🪛 dotenv-linter (4.0.0)
packages/server/.env.example
[warning] 15-15: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 19-19: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 20-20: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 20-20: [UnorderedKey] The LOGCHIMP_PILOT_URL key should go before the LOGCHIMP_SIGNATURE_TOKEN key
(UnorderedKey)
packages/theme/.env.example
[warning] 2-2: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
🪛 GitHub Check: CodeQL
packages/server/src/ee/routes/v1/boards.ts
[failure] 45-45: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
This route handler performs authorization, but is not rate-limited.
[failure] 45-45: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
[failure] 46-46: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
This route handler performs authorization, but is not rate-limited.
[failure] 46-46: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
[failure] 50-50: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
This route handler performs authorization, but is not rate-limited.
[failure] 51-51: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
[failure] 52-52: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.
[failure] 58-58: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
This route handler performs authorization, but is not rate-limited.
[failure] 59-59: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
[failure] 60-60: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
[failure] 64-64: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
This route handler performs authorization, but is not rate-limited.
[failure] 64-64: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
packages/server/src/ee/routes/v1/posts.ts
[failure] 29-29: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
packages/server/src/ee/routes/v1/roadmaps.ts
[failure] 13-13: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
This route handler performs authorization, but is not rate-limited.
[failure] 13-13: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.
[failure] 18-18: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
This route handler performs authorization, but is not rate-limited.
[failure] 19-19: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
[failure] 20-20: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
[failure] 22-22: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
This route handler performs authorization, but is not rate-limited.
[failure] 22-22: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.
[failure] 25-25: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
[failure] 26-26: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
[failure] 27-27: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
This route handler performs authorization, but is not rate-limited.
packages/server/src/ee/routes/v1/roles.ts
[failure] 19-19: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
This route handler performs authorization, but is not rate-limited.
[failure] 19-19: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
[failure] 29-29: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
This route handler performs authorization, but is not rate-limited.
[failure] 29-29: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
[failure] 31-31: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
This route handler performs authorization, but is not rate-limited.
[failure] 31-31: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
[failure] 31-31: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.
🪛 Gitleaks (8.28.0)
packages/server/.env.example
[high] 19-20: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (23)
packages/theme/.env.example (1)
2-2: Good addition; false positive warning.The new
VITE_WEBAPP_URLenvironment variable follows the established naming and quoting conventions consistent with other variables in the file. The localhost default is appropriate for local development.The dotenv-linter warning about quote characters is a false positive—all variables in the file use the same double-quote style. No action needed.
packages/theme/src/store/settings.ts (1)
16-16: LGTM! Verification confirms server-side properly populates the field.The server-side siteSettings controller at
packages/server/src/controllers/settings/siteSettings.tscorrectly computeshasValidLicenseviacheckLicense()and includes it in the response (line 28). The frontend store initialization withhasValidLicense: falseis the proper secure default, and the existingupdate()action will correctly merge the server's computed value when settings are fetched. The implementation is complete and correct.packages/theme/src/ee/components/dashboard/boards/SearchBoardDropdown/Dropdown.vue (1)
12-12: LGTM! Disabled state styling enhances UX and accessibility.The disabled state styling is well-implemented:
disabled:bg-neutral-100anddisabled:cursor-not-allowedprovide clear visual feedback on the triggergroup-disabled:text-neutral-400andgroup-disabled:stroke-neutral-500appropriately lighten the text and icon when the parent group is disabled- The Tailwind v4 syntax is correct and the
group-disabled:*variants work properly with the parent'sgroupclass and:disabledbindingThese changes align well with the license gating feature, providing users with clear visual cues when the component is disabled due to licensing restrictions.
Also applies to: 33-33, 39-39
packages/server/src/errorResponse.json (1)
17-19: LGTM!The new license verification error message is appropriately structured and provides clear feedback for failed license validation.
packages/server/src/ee/routes/v1/license.ts (1)
1-10: LGTM!Clean route implementation for the license check endpoint. The route correctly delegates to the controller without applying license guard middleware (which would create a circular dependency).
packages/server/src/ee/routes/v1/roadmaps.ts (1)
11-30: LGTM!The middleware ordering is correct:
licenseGuard→authRequired→roadmapExists(where applicable) → controller. This ensures license validation occurs before authentication and resource checks.packages/server/src/ee/routes/v1/posts.ts (1)
17-53: LGTM!The middleware chain correctly applies
licenseGuardbeforeauthRequiredfor comment operations, and standalone for activity retrieval. The@ts-expect-errorsuppressions are handling Express middleware typing complexities.packages/server/src/ee/routes/v1/boards.ts (1)
16-61: LGTM!The
licenseGuardmiddleware is consistently applied across all board routes with correct ordering: license validation precedes authentication and resource validation checks.packages/server/src/ee/middleware/licenseGuard.ts (1)
8-34: LGTM!The middleware properly handles both error responses (with
codefield) and inactive license states, with appropriate try-catch error handling for Express 4.x. The 403/500 status code usage is correct.packages/server/src/ee/routes/v1/roles.ts (1)
17-51: LGTM!The
licenseGuardmiddleware is correctly positioned before authentication and resource validation across all role routes, maintaining a consistent and secure middleware chain.packages/server/src/utils/logchimpConfig.ts (5)
13-13: LGTM on Config interface additions.The new fields for version, license configuration, and cache prefix are properly typed and integrate well with the existing Config structure.
Also applies to: 24-28, 38-38
132-136: LGTM on file-based license configuration.The license configuration reading from
config.licensefollows the established pattern and properly defaults the pilot URL.
149-149: LGTM on cachePrefix configuration.The cache prefix configuration correctly reads from file config without a default, allowing the environment config default to be used when not specified.
191-196: LGTM on environment-based license configuration.The environment variable mapping correctly reads license configuration and provides appropriate defaults.
208-208: LGTM on cachePrefix default.The default value
"logchimp:"is a sensible choice for preventing key collisions in shared cache instances.packages/server/src/ee/services/license/checkLicense.ts (8)
1-14: LGTM on imports and configuration setup.The imports are properly organized and the configuration is correctly retrieved at module initialization.
16-26: LGTM on helper functions and constants.The cache key generation and empty response constant are well-structured.
28-43: Memory cache structure is well-designed.The memory cache implementation with TTL is appropriate for the use case.
45-78: LGTM on checkLicense implementation.The function correctly implements caching, concurrency coalescing, and configuration validation. The warning for missing
licenseSignaturewithout early return allows for more detailed error context later in the flow.
80-144: LGTM on performLicenseCheck flow.The license check flow correctly handles caching, server ping, and decryption with appropriate error handling and fallbacks.
Also applies to: 146-149
151-160: Memory cache TTL is intentionally shorter.The 1-hour memory cache TTL is shorter than the 24-hour external cache TTL (from
LOGCHIMP_LICENSE_CACHE_TTL_IN_SEC). This appears intentional to allow faster license status updates in memory.
229-239: LGTM on deployment provider detection.The platform detection logic correctly identifies common deployment providers and provides a sensible fallback.
241-254: LGTM on cache clearing implementation.The cache clearing function correctly handles both memory and external cache with appropriate error handling.
| LOGCHIMP_LICENSE_KEY="" | ||
| # Signature token for the LogChimp License API (used for self-hosted) | ||
| # Along with license key, a signature token will be provide to you. | ||
| # This ensure you and only you can communicate with the LogChimp License API for your license key. |
There was a problem hiding this comment.
Fix grammatical error in comment.
Change "This ensure" to "This ensures" for proper subject-verb agreement.
-# This ensure you and only you can communicate with the LogChimp License API for your license key.
+# This ensures you and only you can communicate with the LogChimp License API for your license key.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # This ensure you and only you can communicate with the LogChimp License API for your license key. | |
| # This ensures you and only you can communicate with the LogChimp License API for your license key. |
🤖 Prompt for AI Agents
In packages/server/.env.example around line 18, the comment contains a
grammatical error: change "This ensure you and only you can communicate with the
LogChimp License API for your license key." to "This ensures you and only you
can communicate with the LogChimp License API for your license key." so the verb
agrees with the singular subject.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
packages/theme/src/ee/pages/dashboard/roadmaps/Index.vue (1)
11-21: Button should be disabled when license is invalid.Similar to the roles page, the Create Roadmap button remains enabled when
hasValidLicenseis false (as long as the user hasroadmap:createpermission). Clicking the button will triggercreateRoadmapHandler(line 65), which will likely fail on the backend without a valid license, leading to a poor user experience. The button should be disabled when the license is invalid.Apply this diff:
<Button type="primary" - :disabled="createRoadmapButtonDisabled" + :disabled="createRoadmapButtonDisabled || !hasValidLicense" :loading="createRoadmapButtonLoading" @click="createRoadmapHandler" >packages/server/src/ee/services/license/checkLicense.ts (3)
210-223: Verify JWT expiration handling.
jwt.verify()automatically validates theexpclaim if present in the JWT payload. However, ensure the license server includes anexpclaim in the token. If the server uses a custom expiration field or omitsexp, add explicit validation.Consider wrapping the verification to handle expiration explicitly:
- const decoded = jwt.verify( - encryptedPayload, - config.licenseSignature, - ) as ICheckLicenseDecryptedPayload; + let decoded: ICheckLicenseDecryptedPayload; + try { + decoded = jwt.verify( + encryptedPayload, + config.licenseSignature, + ) as ICheckLicenseDecryptedPayload; + } catch (error) { + if (error instanceof jwt.TokenExpiredError) { + await clearCache().catch(() => {}); + throw new Error("License token has expired"); + } + throw error; + }Run this script to check if the license server documentation specifies the
expclaim:#!/bin/bash # Search for license server API documentation or response examples rg -n -C5 "exp|expir|ttl" --type-add 'doc:*.{md,txt,rst}' --type doc
40-40: Fix type mismatch in pendingLicenseCheck.The type is incompatible with
checkLicense's return type. Line 60 can returnIApiErrorResponsefrom memory cache, butpendingLicenseCheckexcludes that type.Apply this diff:
-let pendingLicenseCheck: Promise<ICheckLicenseDecryptedPayload> | null = null; +let pendingLicenseCheck: Promise<ICheckLicenseDecryptedPayload | IApiErrorResponse> | null = null;
142-142: Remove debug console.log statement.Error logging is already handled at lines 138-141. Remove before merging.
Apply this diff:
logger.error({ message: "failed to decrypt license payload", error: e, }); - console.log(e); return EMPTY_LICENSE_RESPONSE;
🧹 Nitpick comments (2)
packages/theme/src/ee/components/LicenseRequired.vue (1)
34-34: Consider extracting the long description text.The description text spans multiple sentences and includes an email address. For easier maintenance and potential i18n support, consider moving it to a constant or the component's props.
packages/server/src/ee/services/license/checkLicense.ts (1)
148-157: Consider extracting memory cache TTL to a constant.Memory cache TTL is hardcoded to 1 hour (line 152), while external cache uses
LOGCHIMP_LICENSE_CACHE_TTL_IN_SEC(24 hours). This inconsistency may be intentional, but using named constants improves clarity and maintainability.Define a constant in
constants.ts:export const LOGCHIMP_LICENSE_MEMORY_CACHE_TTL_IN_MS = 60 * 60 * 1000; // 1 hourThen apply this diff:
function updateMemoryCache( payload: ICheckLicenseDecryptedPayload | IApiErrorResponse, now: number, ): void { - const expiresAt = now + 60 * 60 * 1000; + const expiresAt = now + LOGCHIMP_LICENSE_MEMORY_CACHE_TTL_IN_MS; memoryCache = { payload, expiresAt, }; }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
packages/server/src/controllers/settings/siteSettings.ts(2 hunks)packages/server/src/ee/controllers/v1/checkLicense.ts(1 hunks)packages/server/src/ee/services/license/checkLicense.ts(1 hunks)packages/theme/src/components/dashboard/posts/PostEditor/index.vue(3 hunks)packages/theme/src/ee/components/LicenseRequired.vue(1 hunks)packages/theme/src/ee/pages/dashboard/boards/Index.vue(1 hunks)packages/theme/src/ee/pages/dashboard/roadmaps/Index.vue(1 hunks)packages/theme/src/ee/pages/dashboard/settings/billing.vue(2 hunks)packages/theme/src/ee/pages/dashboard/settings/roles/Index.vue(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/server/src/controllers/settings/siteSettings.ts
- packages/theme/src/ee/pages/dashboard/settings/billing.vue
- packages/theme/src/components/dashboard/posts/PostEditor/index.vue
- packages/server/src/ee/controllers/v1/checkLicense.ts
- packages/theme/src/ee/pages/dashboard/boards/Index.vue
🧰 Additional context used
🧬 Code graph analysis (1)
packages/server/src/ee/services/license/checkLicense.ts (4)
packages/server/src/utils/logchimpConfig.ts (1)
configManager(247-247)packages/types/src/license.ts (3)
ICheckLicenseDecryptedPayload(15-21)ICheckLicenseResponseBody(10-13)ICheckLicenseRequestBody(1-8)packages/types/src/common.ts (1)
IApiErrorResponse(36-41)packages/server/src/constants.ts (1)
LOGCHIMP_LICENSE_CACHE_TTL_IN_SEC(10-10)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: API / Integration Tests - OS ubuntu-24.04 - Node v22 - Postgres v18
- GitHub Check: API / Integration Tests - OS ubuntu-24.04 - Node v22 - Postgres v14
- GitHub Check: API / Integration Tests - OS ubuntu-24.04 - Node v22 - Postgres v17
- GitHub Check: API / Integration Tests - OS ubuntu-24.04 - Node v22 - Postgres v12.18
- GitHub Check: API / Integration Tests - OS ubuntu-24.04 - Node v22 - Postgres v16
- GitHub Check: test
- GitHub Check: lighthouse
🔇 Additional comments (4)
packages/theme/src/ee/components/LicenseRequired.vue (2)
12-13: Reactivity properly restored.The use of
storeToRefscorrectly maintains Pinia reactivity, ensuring the component re-renders whenhasValidLicensechanges. This addresses the critical issue from the previous review.
15-21: LGTM: Dev warning is helpful.The development-mode console warning provides clear feedback to developers working with enterprise features locally.
packages/theme/src/ee/pages/dashboard/settings/roles/Index.vue (1)
54-55: Reactivity properly maintained.The correct use of
storeToRefsensureshasValidLicenseremains reactive, addressing the previous review concern.packages/theme/src/ee/pages/dashboard/roadmaps/Index.vue (1)
55-56: Reactivity properly maintained.The use of
storeToRefscorrectly preserves reactivity forhasValidLicense, ensuring the UI updates when the license state changes.
| <UpgradeTooltip :has-valid-license="hasValidLicense"> | ||
| <Button | ||
| type="primary" | ||
| :loading="createRoleButtonLoading" | ||
| :disabled="createRoleButtonDisabled" | ||
| @click="createRoleHandler" | ||
| > | ||
| Create | ||
| <LicenseCrown v-if="!hasValidLicense" /> | ||
| </Button> | ||
| </UpgradeTooltip> |
There was a problem hiding this comment.
Consider disabling the button when license is invalid.
The Create button remains enabled (if the user has role:create permission) even when hasValidLicense is false. This allows users to click and trigger createRoleHandler (line 64), which will likely fail on the backend, resulting in a confusing experience. Consider updating the :disabled binding to also check the license state.
Apply this diff to disable the button when license is invalid:
<Button
type="primary"
:loading="createRoleButtonLoading"
- :disabled="createRoleButtonDisabled"
+ :disabled="createRoleButtonDisabled || !hasValidLicense"
@click="createRoleHandler"
>🤖 Prompt for AI Agents
In packages/theme/src/ee/pages/dashboard/settings/roles/Index.vue around lines 9
to 19, the Create Button is only disabled based on createRoleButtonDisabled and
still clickable when hasValidLicense is false; update the :disabled binding so
the button is disabled when either createRoleButtonDisabled is true OR
hasValidLicense is false (i.e., :disabled="createRoleButtonDisabled ||
!hasValidLicense") so users cannot click Create when the license is invalid
(keep the LicenseCrown rendering as-is).
mittalyashu
force-pushed
the
feat/license-validation
branch
from
5bbcbfd to
336f9e1
Compare
|
Rebased the PR |
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (8)
packages/server/.env.example (2)
17-17: Fix grammatical error in comment.Change "will be provide" to "will be provided".
-# Along with license key, a signature token will be provide to you. +# Along with license key, a signature token will be provided to you.
18-18: Fix grammatical error in comment.Change "This ensure" to "This ensures" for proper subject-verb agreement.
-# This ensure you and only you can communicate with the LogChimp License API for your license key. +# This ensures you and only you can communicate with the LogChimp License API for your license key.packages/server/src/utils/logchimpConfig.ts (2)
10-10: Address code duplication of DEFAULT_LOGCHIMP_PILOT_URL.The constant
DEFAULT_LOGCHIMP_PILOT_URLis defined in both the server package (here) and the theme package (packages/theme/src/constants.ts). Consider moving this to a shared location to maintain a single source of truth.Based on learnings
232-247: Remove commented-out code.The commented-out
getEnvhelper method should be removed. If this functionality is needed in the future, it can be retrieved from version control.- - // private getEnv = <K extends keyof NodeJS.ProcessEnv>(key: K, fallback?: NodeJS.ProcessEnv[K]): NodeJS.ProcessEnv[K] => { - // const value = process.env[key] as NodeJS.ProcessEnv[K] | undefined; - // - // if (value === undefined) { - // // handle fallback falsy cases that should still be used as value - // if (fallback === false || fallback === "" || fallback === 0) { - // return fallback; - // } - // if (fallback) { - // return fallback; - // } - // throw new Error(`Missing environment variable: ${key}.`); - // } - // - // return value; - // }; }packages/server/src/ee/services/license/checkLicense.ts (3)
40-40: Align pendingLicenseCheck type with function return type.The
pendingLicenseCheckis typed asPromise<ICheckLicenseDecryptedPayload> | null, but it's returned directly fromcheckLicense()on line 64, which promisesPromise<ICheckLicenseDecryptedPayload | IApiErrorResponse>. While this is type-safe due to covariance, it creates an inconsistency—the memory cache (line 30) can holdIApiErrorResponse, but the pending check cannot.Apply this diff:
-let pendingLicenseCheck: Promise<ICheckLicenseDecryptedPayload> | null = null; +let pendingLicenseCheck: Promise<ICheckLicenseDecryptedPayload | IApiErrorResponse> | null = null;
210-226: Verify JWT expiration handling.The
jwt.verify()call will automatically validate theexpclaim if present in the JWT and throwTokenExpiredErrorif expired. However, ensure that:
- The license server includes an
expclaim in issued JWTs- Expired token errors are handled appropriately (currently they're caught generically and return EMPTY_LICENSE_RESPONSE)
Run this script to check if TokenExpiredError is handled elsewhere in the codebase:
#!/bin/bash # Search for TokenExpiredError handling or jwt expiration logic rg -nP "TokenExpiredError|jwt.*expir|exp.*claim" --type ts -C 2
137-145: Remove debug console.log statement.Line 142 contains a debug
console.log(e)that should be removed before merging to production. Error logging is already handled by the logger on lines 138-141.Apply this diff:
} catch (e) { logger.error({ message: "failed to decrypt license payload", error: e, }); - console.log(e); return EMPTY_LICENSE_RESPONSE; }packages/types/src/license.ts (1)
10-13: Timestamp type remains inconsistent with JSON parsing.The
timestampfield is typed asDate, butresponse.json()in the service (packages/server/src/ee/services/license/checkLicense.ts:194) returns strings for date fields, not Date objects. The type assertion doesn't perform runtime conversion, so the actual runtime value will be a string, causing type mismatches.Apply this diff:
export interface ICheckLicenseResponseBody { encrypted_payload: string; - timestamp: Date; + timestamp: string; }
🧹 Nitpick comments (6)
packages/types/src/site/settings.ts (1)
11-11: Consider adding JSDoc for the new field.Adding documentation would help clarify what constitutes a "valid license" and how this field affects the application behavior.
+ /** Indicates whether the instance has a valid enterprise license */ hasValidLicense: boolean;packages/theme/src/ee/store/settings.ts (2)
14-14: Simplify redundant boolean expression.The expression
response.status === "active" || falseis redundant since the comparison already returns a boolean.- hasValidLicense.value = response.status === "active" || false; + hasValidLicense.value = response.status === "active";
16-18: Add explanation for type assertion and improve type safety.The
@ts-expect-errordirective lacks explanation, and the nested property access is not type-safe.- // @ts-expect-error - const isInvalid = e.response?.data?.code === "LICENSE_VALIDATION_FAILED"; + // Axios error response structure is not typed + const isInvalid = (e as any).response?.data?.code === "LICENSE_VALIDATION_FAILED";Or better, define a proper error type for your API responses.
packages/theme/src/components/EmptyScreen.vue (1)
4-4: Improve icon prop typing.The
icon?: unknowntype is too loose. Use a proper Vue component type for better type safety.+import type { Component } from 'vue'; + interface Props { title: string; - icon?: unknown; + icon?: Component; description?: string; border?: boolean; dashedBorder?: boolean; }packages/types/src/license.ts (1)
1-8: Refine deploymentProvider type to specific literals.The type
string | "unknown"is redundant since"unknown"is already covered bystring. Based on the implementation in packages/server/src/ee/services/license/checkLicense.ts:228-237, this should be a union of specific provider literals for better type safety.Apply this diff:
export interface ICheckLicenseRequestBody { license_key: string; metadata: { version: string; - deploymentProvider: string | "unknown"; + deploymentProvider: "vercel" | "render" | "railway" | "unknown"; }; timestamp: string; }packages/theme/src/ee/components/UpgradeTooltip.vue (1)
23-25: Simplify redundant disabled condition.The
:disabledbinding checksIS_PROD && !hasValidLicense, but this code is within thev-elsebranch (line 21) wherehasValidLicenseis guaranteed to befalse. The condition simplifies to justIS_PROD.Apply this diff:
<TooltipTrigger - :disabled="IS_PROD && !hasValidLicense" + :disabled="IS_PROD" >
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (38)
docker/community/docker-compose.yml(1 hunks)packages/server/.env.example(1 hunks)packages/server/src/constants.ts(1 hunks)packages/server/src/controllers/settings/siteSettings.ts(2 hunks)packages/server/src/ee/controllers/v1/checkLicense.ts(1 hunks)packages/server/src/ee/middleware/licenseGuard.ts(1 hunks)packages/server/src/ee/routes/v1/boards.ts(1 hunks)packages/server/src/ee/routes/v1/license.ts(1 hunks)packages/server/src/ee/routes/v1/posts.ts(1 hunks)packages/server/src/ee/routes/v1/roadmaps.ts(1 hunks)packages/server/src/ee/routes/v1/roles.ts(2 hunks)packages/server/src/ee/services/license/checkLicense.ts(1 hunks)packages/server/src/environment.d.ts(1 hunks)packages/server/src/errorResponse.json(1 hunks)packages/server/src/routes/v1/index.ts(2 hunks)packages/server/src/utils/logchimpConfig.ts(7 hunks)packages/theme/.env.example(1 hunks)packages/theme/inject-app-env.sh(1 hunks)packages/theme/src/components/EmptyScreen.vue(1 hunks)packages/theme/src/components/dashboard/posts/PostEditor/index.vue(3 hunks)packages/theme/src/constants.ts(2 hunks)packages/theme/src/ee/components/LicenseRequired.vue(1 hunks)packages/theme/src/ee/components/UpgradeTooltip.vue(1 hunks)packages/theme/src/ee/components/dashboard/boards/SearchBoardDropdown/Dropdown.vue(2 hunks)packages/theme/src/ee/components/dashboard/roadmap/SearchRoadmapDropdown/Dropdown.vue(2 hunks)packages/theme/src/ee/components/icons/LicenseCrown.vue(1 hunks)packages/theme/src/ee/components/license/LocalLicenseAlert.vue(1 hunks)packages/theme/src/ee/modules/settings.ts(1 hunks)packages/theme/src/ee/pages/dashboard/boards/Index.vue(1 hunks)packages/theme/src/ee/pages/dashboard/roadmaps/Index.vue(1 hunks)packages/theme/src/ee/pages/dashboard/settings/billing.vue(2 hunks)packages/theme/src/ee/pages/dashboard/settings/roles/Index.vue(1 hunks)packages/theme/src/ee/store/settings.ts(1 hunks)packages/theme/src/pages/dashboard/posts/Index.vue(1 hunks)packages/theme/src/store/settings.ts(1 hunks)packages/types/src/index.ts(1 hunks)packages/types/src/license.ts(1 hunks)packages/types/src/site/settings.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (24)
- packages/server/src/ee/routes/v1/license.ts
- packages/theme/inject-app-env.sh
- packages/theme/src/constants.ts
- packages/theme/src/store/settings.ts
- packages/theme/src/ee/components/dashboard/roadmap/SearchRoadmapDropdown/Dropdown.vue
- packages/theme/src/ee/components/dashboard/boards/SearchBoardDropdown/Dropdown.vue
- packages/theme/src/ee/components/license/LocalLicenseAlert.vue
- packages/theme/src/ee/pages/dashboard/settings/roles/Index.vue
- packages/server/src/ee/routes/v1/roles.ts
- packages/theme/src/components/dashboard/posts/PostEditor/index.vue
- packages/server/src/environment.d.ts
- packages/theme/src/ee/pages/dashboard/boards/Index.vue
- packages/theme/src/ee/pages/dashboard/settings/billing.vue
- packages/theme/src/ee/pages/dashboard/roadmaps/Index.vue
- packages/theme/src/ee/modules/settings.ts
- docker/community/docker-compose.yml
- packages/server/src/routes/v1/index.ts
- packages/server/src/ee/middleware/licenseGuard.ts
- packages/server/src/ee/routes/v1/boards.ts
- packages/server/src/ee/routes/v1/posts.ts
- packages/server/src/ee/routes/v1/roadmaps.ts
- packages/theme/src/pages/dashboard/posts/Index.vue
- packages/types/src/index.ts
- packages/server/src/controllers/settings/siteSettings.ts
🧰 Additional context used
🧬 Code graph analysis (3)
packages/server/src/ee/services/license/checkLicense.ts (4)
packages/server/src/utils/logchimpConfig.ts (1)
configManager(250-250)packages/types/src/license.ts (3)
ICheckLicenseDecryptedPayload(15-21)ICheckLicenseResponseBody(10-13)ICheckLicenseRequestBody(1-8)packages/types/src/common.ts (1)
IApiErrorResponse(36-41)packages/server/src/constants.ts (1)
LOGCHIMP_LICENSE_CACHE_TTL_IN_SEC(11-11)
packages/theme/src/ee/store/settings.ts (1)
packages/theme/src/ee/modules/settings.ts (1)
SettingsEE(6-18)
packages/server/src/utils/logchimpConfig.ts (1)
packages/theme/src/constants.ts (1)
DEFAULT_LOGCHIMP_PILOT_URL(17-18)
🪛 dotenv-linter (4.0.0)
packages/server/.env.example
[warning] 15-15: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 19-19: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 20-20: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 20-20: [UnorderedKey] The LOGCHIMP_PILOT_URL key should go before the LOGCHIMP_SIGNATURE_TOKEN key
(UnorderedKey)
packages/theme/.env.example
[warning] 2-2: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
🪛 Gitleaks (8.29.0)
packages/server/.env.example
[high] 19-20: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (7)
packages/theme/.env.example (1)
2-2: Correctly adds the new environment variable.The new
VITE_WEBAPP_URLvariable follows the established pattern in the file and aligns with the license validation feature (passing the webapp URL to the theme service). The static analysis warning about quote characters is a false positive—all variables in this file use the same quoted format.packages/types/src/site/settings.ts (1)
11-11: Verify server always provideshasValidLicense.The field is typed as non-nullable
boolean, which means the server must always return a value for this property. Ensure that all code paths in the license validation service and settings API return a boolean value (defaulting tofalsewhen license validation fails or is unavailable).packages/server/src/constants.ts (1)
9-11: LGTM!The cache TTL constant is well-defined with a clear comment explaining the duration.
packages/server/src/utils/logchimpConfig.ts (1)
5-5: License configuration integration looks good.The license-related configuration additions are well-integrated into the existing configuration management pattern. The version field, license keys, signature tokens, and pilot URL are properly handled in both file-based and environment-based configurations.
Also applies to: 13-13, 25-28, 134-137, 193-198, 228-228
packages/server/src/errorResponse.json (1)
18-20: LGTM!The license verification error message is properly structured and follows the existing error response pattern.
packages/theme/src/ee/components/icons/LicenseCrown.vue (1)
1-21: LGTM!The icon component is well-structured with proper prop typing and defaults. The conditional CSS classes provide flexible styling.
packages/theme/src/ee/components/LicenseRequired.vue (1)
1-42: Component structure looks good, but verify store integration.The component correctly uses
storeToRefsto maintain reactivity and implements appropriate conditional rendering for license gating. However, ensure you address the critical issue inpackages/theme/src/ee/store/settings.tsregarding the improper use ofonMountedin the Pinia store, as this component depends on that store.
| } catch (e) { | ||
| logger.error({ | ||
| message: e, | ||
| }); |
There was a problem hiding this comment.
Fix error logging field.
Line 23 logs the exception using message: e, but e is an Error object that may not serialize properly. Use the error field instead to align with the logger's expected structure and ensure stack traces are captured.
Apply this diff:
} catch (e) {
logger.error({
- message: e,
+ error: e,
+ message: "Failed to check license",
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } catch (e) { | |
| logger.error({ | |
| message: e, | |
| }); | |
| } catch (e) { | |
| logger.error({ | |
| error: e, | |
| message: "Failed to check license", | |
| }); |
🤖 Prompt for AI Agents
In packages/server/src/ee/controllers/v1/checkLicense.ts around lines 21 to 24,
the catch block logs the thrown Error using message: e which may not serialize
the Error or include its stack; change the logger call to use the error field
(e.g., logger.error({ error: e })) so the logger captures the Error object and
stack trace; ensure the catch variable is passed as the Error object (cast if
necessary) and remove or replace the message field accordingly.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
packages/server/src/ee/services/license/checkLicense.ts (1)
156-156: Remove leftover debug console.log statement.This debug statement was flagged in a previous review as addressed but is still present. It should be removed before merging to production, as the error is already properly logged on lines 152-155.
Apply this diff:
logger.error({ message: "failed to decrypt license payload", error: e, }); - console.log(e); return EMPTY_LICENSE_RESPONSE;
🧹 Nitpick comments (4)
packages/server/src/ee/services/license/checkLicense.ts (4)
110-115: Improve type narrowing for API error detection.The check
"code" in datais used to narrow the type toIApiErrorResponse, but this is fragile sincecodeis not a required discriminator in TypeScript. Consider using a more explicit type guard or checking for the presence ofencrypted_payloadfirst.Apply this diff for clearer intent:
try { const data = await pingLicenseServer(); - if ("code" in data || !data.encrypted_payload) { + if (!data.encrypted_payload || "code" in data) { updateMemoryCache(data as IApiErrorResponse, now); return EMPTY_LICENSE_RESPONSE; }
162-171: Consider extracting memory cache TTL to a named constant.The memory cache TTL is hardcoded as
60 * 60 * 1000(1 hour). For consistency withLOGCHIMP_LICENSE_CACHE_TTL_IN_SECand improved maintainability, consider extracting this to a named constant.For example, add at the top of the file:
const MEMORY_CACHE_TTL_MS = 60 * 60 * 1000; // 1 hourThen use it on line 166:
function updateMemoryCache( payload: ILicenseServiceResponse | IApiErrorResponse, now: number, ): void { - const expiresAt = now + 60 * 60 * 1000; + const expiresAt = now + MEMORY_CACHE_TTL_MS; memoryCache = { payload, expiresAt, }; }
176-184: Remove or document commented-outmachine_signaturefield.Line 178 contains a commented-out field. If
machine_signatureis not currently used, remove the comment. If it's planned for future use, add a TODO comment explaining why it's commented out and when it will be enabled.Apply this diff to remove the commented line:
const requestBody: ICheckLicenseRequestBody = { license_key: config.licenseKey, - // machine_signature: config.machineSignature, metadata: { version: config.version, deploymentProvider, }, timestamp: new Date().toISOString(), };
242-252: Incomplete deployment provider detection.The
getDeploymentProviderfunction only detects Vercel, Render, and Railway, butTDeploymentProvideralso includes AWS, GCP, Azure, and Heroku. Consider adding detection for the missing providers or documenting why they're not yet supported.For reference, common environment variables:
- AWS:
AWS_EXECUTION_ENVorAWS_LAMBDA_FUNCTION_NAME- GCP:
GOOGLE_CLOUD_PROJECTorK_SERVICE- Azure:
WEBSITE_SITE_NAMEorAZURE_FUNCTIONS_ENVIRONMENT- Heroku:
DYNOApply this diff to add detection for the missing providers:
function getDeploymentProvider(): TDeploymentProvider { if (process.env.VERCEL) { return "vercel"; } else if (process.env.RENDER) { return "render"; } else if (process.env.RAILWAY_SERVICE_ID) { return "railway"; + } else if (process.env.DYNO) { + return "heroku"; + } else if (process.env.AWS_EXECUTION_ENV || process.env.AWS_LAMBDA_FUNCTION_NAME) { + return "aws"; + } else if (process.env.GOOGLE_CLOUD_PROJECT || process.env.K_SERVICE) { + return "gcp"; + } else if (process.env.WEBSITE_SITE_NAME || process.env.AZURE_FUNCTIONS_ENVIRONMENT) { + return "azure"; } else { return "unknown"; } }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
packages/server/src/ee/controllers/v1/roadmaps/updateRoadmap.ts(3 hunks)packages/server/src/ee/middleware/licenseGuard.ts(1 hunks)packages/server/src/ee/routes/v1/boards.ts(1 hunks)packages/server/src/ee/routes/v1/posts.ts(1 hunks)packages/server/src/ee/routes/v1/roadmaps.ts(1 hunks)packages/server/src/ee/routes/v1/roles.ts(1 hunks)packages/server/src/ee/services/license/checkLicense.ts(1 hunks)packages/server/src/errorResponse.json(1 hunks)packages/server/src/middlewares/roadmapExists.ts(3 hunks)packages/server/src/routes/v1/roadmaps.ts(2 hunks)packages/types/src/license.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/server/src/errorResponse.json
- packages/server/src/ee/routes/v1/roles.ts
🧰 Additional context used
🧬 Code graph analysis (8)
packages/server/src/ee/services/license/checkLicense.ts (4)
packages/server/src/utils/logchimpConfig.ts (1)
configManager(250-250)packages/types/src/license.ts (6)
ILicenseServiceResponse(39-41)ICheckLicenseResponseBody(26-29)ICheckLicenseRequestBody(17-24)ICheckLicenseDecryptedPayload(31-37)TDeploymentProvider(1-9)TLicenseSubscriptionType(11-15)packages/types/src/common.ts (1)
IApiErrorResponse(36-41)packages/server/src/constants.ts (1)
LOGCHIMP_LICENSE_CACHE_TTL_IN_SEC(11-11)
packages/server/src/ee/middleware/licenseGuard.ts (3)
packages/types/src/common.ts (1)
IApiErrorResponse(36-41)packages/types/src/license.ts (1)
TLicenseSubscriptionType(11-15)packages/server/src/ee/services/license/checkLicense.ts (1)
checkLicense(47-80)
packages/server/src/routes/v1/roadmaps.ts (2)
packages/types/src/roadmap.ts (1)
IGetRoadmapByUrlRequestParam(29-31)packages/server/src/middlewares/roadmapExists.ts (1)
roadmapExists(17-63)
packages/server/src/middlewares/roadmapExists.ts (1)
packages/types/src/roadmap.ts (1)
IGetRoadmapByUrlRequestParam(29-31)
packages/server/src/ee/controllers/v1/roadmaps/updateRoadmap.ts (1)
packages/types/src/roadmap.ts (2)
IUpdateRoadmapRequestBody(51-57)IRoadmapPrivate(14-18)
packages/server/src/ee/routes/v1/boards.ts (4)
packages/types/src/board.ts (4)
TFilterBoardRequestQuery(30-30)IGetBoardsRequestQuery(20-24)IGetBoardByUrlRequestParams(36-38)ISearchBoardRequestParams(90-92)packages/server/src/ee/middleware/licenseGuard.ts (1)
withLicenseGuard(11-57)packages/server/src/ee/middleware/boardExists.ts (1)
boardExists(17-53)packages/server/src/middlewares/auth/index.ts (1)
authRequired(5-5)
packages/server/src/ee/routes/v1/posts.ts (3)
packages/types/src/posts/activity.ts (5)
IGetPostActivityRequestParam(24-26)IGetPostActivityRequestQuery(28-31)TCreatePostCommentRequestParam(37-37)IUpdatePostCommentRequestParam(49-52)TDeletePostCommentRequestParam(64-64)packages/server/src/ee/middleware/licenseGuard.ts (1)
withLicenseGuard(11-57)packages/server/src/middlewares/auth/index.ts (1)
authRequired(5-5)
packages/server/src/ee/routes/v1/roadmaps.ts (3)
packages/server/src/middlewares/auth/index.ts (1)
authRequired(5-5)packages/server/src/ee/middleware/licenseGuard.ts (1)
withLicenseGuard(11-57)packages/server/src/middlewares/roadmapExists.ts (1)
roadmapExists(17-63)
🪛 GitHub Actions: Lighthouse test
packages/server/src/ee/routes/v1/boards.ts
[error] 20-20: TS2769: No overload matches this call.
🪛 GitHub Actions: Playwright Tests
packages/server/src/ee/routes/v1/boards.ts
[error] 20-20: TS2769: No overload matches this call.
🪛 GitHub Actions: Test
packages/server/src/ee/routes/v1/boards.ts
[error] 20-20: TS2769: No overload matches this call.
🪛 GitHub Check: CodeQL
packages/server/src/ee/routes/v1/boards.ts
[failure] 20-22: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
[failure] 26-28: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
[failure] 33-35: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
[failure] 40-42: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
[failure] 47-47: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
This route handler performs authorization, but is not rate-limited.
[failure] 48-50: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
[failure] 54-54: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
This route handler performs authorization, but is not rate-limited.
[failure] 55-57: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
[failure] 61-61: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
This route handler performs authorization, but is not rate-limited.
[failure] 62-62: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
[failure] 63-65: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.
[failure] 70-70: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
This route handler performs authorization, but is not rate-limited.
[failure] 71-71: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
[failure] 72-74: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
[failure] 80-80: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
This route handler performs authorization, but is not rate-limited.
[failure] 81-83: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
packages/server/src/ee/routes/v1/posts.ts
[failure] 28-32: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
[failure] 41-45: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.
[failure] 52-54: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.
[failure] 61-63: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.
packages/server/src/ee/routes/v1/roadmaps.ts
[failure] 15-15: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
This route handler performs authorization, but is not rate-limited.
[failure] 16-18: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.
[failure] 23-23: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
This route handler performs authorization, but is not rate-limited.
[failure] 24-24: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
[failure] 25-27: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
[failure] 31-31: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
This route handler performs authorization, but is not rate-limited.
[failure] 32-34: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
This route handler performs a database access, but is not rate-limited.
[failure] 39-39: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
This route handler performs authorization, but is not rate-limited.
[failure] 40-40: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
[failure] 41-43: Missing rate limiting
This route handler performs a database access, but is not rate-limited.
🔇 Additional comments (3)
packages/server/src/ee/services/license/checkLicense.ts (3)
45-45: Fix the type mismatch inpendingLicenseCheck.The variable is typed as
Promise<ILicenseServiceResponse> | null, butperformLicenseCheck(assigned on line 72) returnsPromise<ILicenseServiceResponse>, which is inconsistent with the fact thatcheckLicensecan returnIApiErrorResponse(via memory cache on line 65). WhileperformLicenseCheckitself only returnsILicenseServiceResponse, the cached payload can be an error response, creating potential type confusion.However, looking more closely, I see that
performLicenseCheckalways returnsILicenseServiceResponse(evenEMPTY_LICENSE_RESPONSEon errors), so the current typing is actually correct for the pending check itself. The issue is that memory cache can store either type but the pending check only stores the success type.Actually, this is correctly typed -
performLicenseCheckreturns onlyILicenseServiceResponse, not the union withIApiErrorResponse. The memory cache handles both types separately.
217-240: Verify JWT expiration handling and error propagation.The
jwt.verify()call will automatically validate theexp(expiration) claim if present in the JWT and throw aTokenExpiredErrorif expired. However, ensure that:
- The license server includes an
expclaim in the JWT payload- Expired tokens trigger cache invalidation (currently, any
jwt.verifyerror propagates toperformLicenseCheckwhich returnsEMPTY_LICENSE_RESPONSE, but doesn't explicitly clear cache)Consider wrapping
jwt.verify()in a try-catch to distinguish between expiration errors and other JWT validation errors, allowing for explicit cache clearing on expiration.Apply this diff to add explicit expiration error handling:
if (!config.licenseSignature) { throw new Error("License signature not configured"); } - const decoded = jwt.verify( - encryptedPayload, - config.licenseSignature, - ) as ICheckLicenseDecryptedPayload; + let decoded: ICheckLicenseDecryptedPayload; + try { + decoded = jwt.verify( + encryptedPayload, + config.licenseSignature, + ) as ICheckLicenseDecryptedPayload; + } catch (error) { + if (error instanceof jwt.TokenExpiredError) { + await clearCache(); + } + throw error; + } if (decoded.license_key !== config.licenseKey) {
254-284: LGTM!The cache clearing logic properly handles both memory and external cache, with appropriate error logging. The subscription hierarchy mapping is clear and includes a sensible default for unknown types.
| export function withLicenseGuard( | ||
| controllerFn: ( | ||
| req: Request, | ||
| res: Response<IApiErrorResponse>, | ||
| next: NextFunction, | ||
| ) => unknown, | ||
| { | ||
| requiredPlan, | ||
| }: { | ||
| requiredPlan: Array<TLicenseSubscriptionType>; | ||
| }, | ||
| ) { | ||
| return async ( | ||
| req: Request, | ||
| res: Response<IApiErrorResponse>, | ||
| next: NextFunction, | ||
| ) => { | ||
| try { | ||
| const result = await checkLicense(); | ||
|
|
||
| if ("code" in result || result.status !== "active") { | ||
| return res.status(403).send({ | ||
| message: error.middleware.license.verification, | ||
| code: "LICENSE_VALIDATION_FAILED", | ||
| }); | ||
| } | ||
|
|
||
| if (!requiredPlan.includes(result.subscription_type)) { | ||
| return res.status(403).send({ | ||
| message: error.middleware.license.higherPlan, | ||
| code: "LICENSE_INSUFFICIENT_TIER", | ||
| }); | ||
| } | ||
|
|
||
| return controllerFn(req, res, next); | ||
| } catch (e) { | ||
| logger.error({ | ||
| message: e, | ||
| }); | ||
|
|
||
| res.status(500).send({ | ||
| message: error.general.serverError, | ||
| code: "SERVER_ERROR", | ||
| }); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Fix license guard typing to satisfy Express handler signature.
withLicenseGuard currently returns a function typed as (req: Request, res: Response<IApiErrorResponse>, next: NextFunction) => unknown. When the router supplies concrete generics for params/query/body, that signature no longer matches any Express overload, so TypeScript raises TS2769 (the same error reported by CI) unless you sprinkle @ts-expect-error everywhere. Updating the helper to expose itself as a proper RequestHandler keeps the original handler’s generics intact, removes the need for suppressions, and unblocks the build.
-import type { Request, Response, NextFunction } from "express";
-import type {
- IApiErrorResponse,
- TLicenseSubscriptionType,
-} from "@logchimp/types";
+import type { RequestHandler } from "express";
+import type {
+ IApiErrorResponse,
+ TLicenseSubscriptionType,
+} from "@logchimp/types";
@@
-export function withLicenseGuard(
- controllerFn: (
- req: Request,
- res: Response<IApiErrorResponse>,
- next: NextFunction,
- ) => unknown,
+export function withLicenseGuard(
+ controllerFn: RequestHandler,
{
requiredPlan,
}: {
requiredPlan: Array<TLicenseSubscriptionType>;
},
) {
- return async (
- req: Request,
- res: Response<IApiErrorResponse>,
- next: NextFunction,
- ) => {
+ const guard: RequestHandler = async (req, res, next) => {
try {
const result = await checkLicense();
if ("code" in result || result.status !== "active") {
- return res.status(403).send({
- message: error.middleware.license.verification,
- code: "LICENSE_VALIDATION_FAILED",
- });
+ const verificationError: IApiErrorResponse = {
+ message: error.middleware.license.verification,
+ code: "LICENSE_VALIDATION_FAILED",
+ };
+ return res.status(403).send(verificationError);
}
if (!requiredPlan.includes(result.subscription_type)) {
- return res.status(403).send({
- message: error.middleware.license.higherPlan,
- code: "LICENSE_INSUFFICIENT_TIER",
- });
+ const tierError: IApiErrorResponse = {
+ message: error.middleware.license.higherPlan,
+ code: "LICENSE_INSUFFICIENT_TIER",
+ };
+ return res.status(403).send(tierError);
}
- return controllerFn(req, res, next);
+ await Promise.resolve(controllerFn(req, res, next));
} catch (e) {
logger.error({
message: e,
});
- res.status(500).send({
- message: error.general.serverError,
- code: "SERVER_ERROR",
- });
+ const serverError: IApiErrorResponse = {
+ message: error.general.serverError,
+ code: "SERVER_ERROR",
+ };
+ return res.status(500).send(serverError);
}
};
+
+ return guard;
}Once this is in place, the route files will type-check without the @ts-expect-error scaffolding, resolving the CI blocker.
🤖 Prompt for AI Agents
In packages/server/src/ee/middleware/licenseGuard.ts around lines 11 to 56, the
returned middleware is typed as a concrete (req,res,next) => unknown which
breaks Express generics and causes TS2769; change the helper to accept and
return an Express RequestHandler with preserved generics (e.g. provide generic
type parameters and use RequestHandler<TParams, TResBody, TReqBody, TQuery> for
controllerFn and the return type) so route-level params/query/body generics
propagate correctly; update the function signature to import and use
RequestHandler from express, keep the internal logic the same but have the
returned function match RequestHandler so TypeScript recognizes it as a valid
Express handler and you can remove ts-expect-error suppressions.
| if (!config.licenseSignature) { | ||
| logger.warn( | ||
| "License signature is not configured. Check your environment variables or 'logchimp.config.json' file.", | ||
| ); | ||
| } |
There was a problem hiding this comment.
Add missing return statement after license signature warning.
When licenseSignature is not configured, the code logs a warning but continues execution. This will cause a runtime error later in decryptPayload (line 220-222) when it checks for the signature. For consistency with the licenseKey check above and to fail fast, this should return EMPTY_LICENSE_RESPONSE.
Apply this diff:
if (!config.licenseSignature) {
logger.warn(
"License signature is not configured. Check your environment variables or 'logchimp.config.json' file.",
);
+ return EMPTY_LICENSE_RESPONSE;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (!config.licenseSignature) { | |
| logger.warn( | |
| "License signature is not configured. Check your environment variables or 'logchimp.config.json' file.", | |
| ); | |
| } | |
| if (!config.licenseSignature) { | |
| logger.warn( | |
| "License signature is not configured. Check your environment variables or 'logchimp.config.json' file.", | |
| ); | |
| return EMPTY_LICENSE_RESPONSE; | |
| } |
🤖 Prompt for AI Agents
In packages/server/src/ee/services/license/checkLicense.ts around lines 57 to
61, the warning for a missing licenseSignature currently only logs and allows
execution to continue; add a return of EMPTY_LICENSE_RESPONSE immediately after
the logger.warn call (mirroring the earlier licenseKey check) so the function
fails fast and avoids downstream runtime errors in decryptPayload.
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (1)
packages/theme/src/ee/pages/dashboard/settings/roles/Index.vue (1)
9-19: Consider disabling the button when license is invalid.The Create button remains enabled (if the user has
role:createpermission) even whenhasValidLicenseis false. This allows users to click and triggercreateRoleHandler(line 63), which will likely fail on the backend, resulting in a confusing experience.Apply this diff to disable the button when the license is invalid:
<Button type="primary" :loading="createRoleButtonLoading" - :disabled="createRoleButtonDisabled" + :disabled="createRoleButtonDisabled || !hasValidLicense" @click="createRoleHandler" >
🧹 Nitpick comments (4)
packages/theme/src/ee/store/settings.ts (1)
15-15: Remove redundant boolean operation.The
|| falseis unnecessary sinceresponse.status === "active"already returns a boolean.Apply this diff:
- hasValidLicense.value = response.status === "active" || false; + hasValidLicense.value = response.status === "active";packages/server/src/ee/routes/v1/boards.ts (3)
16-16: Unusual import path with "do-not-remove" directory name.The import path
../../do-not-remove/middleware/licenseGuardcontains "do-not-remove" which is not a semantic or conventional directory name for production code. This suggests unclear organization or a placeholder structure.Consider renaming to a more semantic path such as
../../middleware/licenseGuardor../../ee/middleware/licenseGuardto better reflect the purpose and maintain code clarity.
32-39: Consider license validation order for efficiency.The middleware execution order places
withLicenseGuardafterboardExists, which means the database is queried to validate the board exists before checking if the user has a valid license. If license validation is likely to fail, this results in unnecessary database operations.Consider whether license validation should occur earlier in the middleware chain (e.g., right after
authRequired) to avoid expensive operations for unlicensed users. The trade-off is between efficiency and providing more specific error messages.Also applies to: 64-82
85-91: Consider if license guard is necessary for deprecated route.The
/boards/check-nameendpoint is marked as deprecated and planned for removal in the next major release, yet it's receiving the full license guard implementation. While this maintains consistency, it adds maintenance burden for code that's already marked for removal.If this endpoint is no longer actively used or will be removed soon, consider whether the license guard addition is necessary.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
.gitmodules(1 hunks).idea/vcs.xml(1 hunks)packages/server/src/controllers/roadmaps/roadmapByUrl.ts(1 hunks)packages/server/src/ee/controllers/v1/checkLicense.ts(1 hunks)packages/server/src/ee/do-not-remove(1 hunks)packages/server/src/ee/routes/v1/boards.ts(1 hunks)packages/server/src/ee/routes/v1/posts.ts(1 hunks)packages/server/src/ee/routes/v1/roadmaps.ts(1 hunks)packages/server/src/ee/routes/v1/roles.ts(2 hunks)packages/server/src/express.d.ts(1 hunks)packages/theme/src/components/dashboard/posts/PostEditor/index.vue(4 hunks)packages/theme/src/ee/pages/dashboard/boards/Index.vue(2 hunks)packages/theme/src/ee/pages/dashboard/roadmaps/Index.vue(2 hunks)packages/theme/src/ee/pages/dashboard/settings/roles/Index.vue(2 hunks)packages/theme/src/ee/store/settings.ts(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- .gitmodules
- packages/server/src/ee/do-not-remove
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/server/src/ee/routes/v1/roadmaps.ts
- packages/server/src/ee/controllers/v1/checkLicense.ts
- packages/server/src/ee/routes/v1/posts.ts
- packages/theme/src/ee/pages/dashboard/roadmaps/Index.vue
- packages/server/src/ee/routes/v1/roles.ts
🧰 Additional context used
🧬 Code graph analysis (3)
packages/server/src/ee/routes/v1/boards.ts (3)
packages/types/src/board.ts (4)
TFilterBoardRequestQuery(30-30)IGetBoardsRequestQuery(20-24)IGetBoardByUrlRequestParams(36-38)ISearchBoardRequestParams(90-92)packages/server/src/ee/middleware/boardExists.ts (1)
boardExists(17-53)packages/server/src/middlewares/auth/index.ts (1)
authRequired(5-5)
packages/server/src/controllers/roadmaps/roadmapByUrl.ts (1)
packages/types/src/roadmap.ts (2)
IGetRoadmapByUrlRequestParam(29-31)IGetRoadmapByUrlResponseBody(33-35)
packages/theme/src/ee/store/settings.ts (1)
packages/theme/src/ee/modules/settings.ts (1)
SettingsEE(6-18)
🔇 Additional comments (13)
.idea/vcs.xml (1)
5-5: Verify IDE configuration file versioning practice.IDE configuration files in
.idea/are typically excluded from version control to prevent per-developer configuration drift. Ensure this aligns with your project's policy and that.idea/vcs.xmlis intentionally committed rather than gitignored.The mapping itself is syntactically correct and appropriately registers the new EE submodule directory with Git in the IDE.
packages/server/src/controllers/roadmaps/roadmapByUrl.ts (1)
7-15: Middleware chain is properly configured.Verification confirms the route definition includes the
roadmapExistsmiddleware beforeroadmapByUrl, and the middleware setsreq.roadmapbefore callingnext(). The controller safely accesses the pre-populated property.packages/server/src/express.d.ts (1)
4-14: All usages ofreq.ctx.roadmaphave been successfully migrated — no references remain in the codebase.However, verification reveals an outstanding type safety issue: the codebase still uses
req.roadmapdirectly (in middleware assignment and controller reads) with@ts-expect-errorsuppressions. The Express Request interface inpackages/server/src/express.d.tsdoes not formally define this property, leaving it untyped.Three files use
req.roadmapwith type-error suppression:
packages/server/src/middlewares/roadmapExists.ts:51— assigns the propertypackages/server/src/controllers/roadmaps/roadmapByUrl.ts:12— reads the propertypackages/server/src/ee/controllers/v1/roadmaps/updateRoadmap.ts:88— reads the propertyThe TypeScript error suppressions should either be:
- Resolved by adding
roadmap?: IRoadmapPrivate;to the Express Request type definition, or- Eliminated by refactoring the code to use a different pattern
Please confirm whether this untyped property usage is intentional or requires a type definition update.
packages/theme/src/ee/pages/dashboard/boards/Index.vue (2)
30-54: LGTM - Reactivity properly maintained.The use of
storeToRefsensures thathasValidLicenseremains reactive and the UI will update when the license state changes.
80-82: LGTM - License info fetched on mount.Fetching license information when the component mounts ensures the UI has up-to-date license state.
packages/theme/src/ee/store/settings.ts (2)
1-11: LGTM - Store initialization is correct.The store is properly set up with appropriate imports, and
hasValidLicensecorrectly defaults tofalse(deny by default).
21-22: LGTM - Appropriate throttle interval.The 5-second throttle prevents excessive API calls while remaining responsive for UI updates.
packages/theme/src/components/dashboard/posts/PostEditor/index.vue (3)
74-96: Verify intended behavior for license-gated fields.The Board and Roadmap fields display license prompts (
UpgradeTooltipwithLicenseCrown) whenhasValidLicenseis false, but the dropdown fields remain interactive. Users can still select boards and roadmaps even without a valid license.Is this intentional (allowing users to see and select options but blocking on save), or should the dropdowns be disabled when the license is invalid?
If the fields should be disabled, consider adding a
:disabledprop to bothSearchBoardDropdownandSearchRoadmapDropdown:</div> <SearchBoardDropdown :board="post.board" + :disabled="!hasValidLicense" @selected="selectBoard" /> </div> <div class="form-column"> <div class="flex items-center justify-between gap-2"> <InputLabel html-for="" class="mb-0"> Roadmap </InputLabel> <UpgradeTooltip v-if="!hasValidLicense" :has-valid-license="hasValidLicense"> <LicenseCrown color="neutral" /> </UpgradeTooltip> </div> <SearchRoadmapDropdown :roadmap="post.roadmap" + :disabled="!hasValidLicense" @selected="selectRoadmap" />Note: This assumes both dropdown components support a
disabledprop. You may need to verify this.
107-141: LGTM - Store integration implemented correctly.The component properly imports and integrates with the settings EE store, using
storeToRefsto maintain reactivity forhasValidLicense.
211-213: LGTM - License info fetched on mount.Consistent with the pattern used in other dashboard pages.
packages/theme/src/ee/pages/dashboard/settings/roles/Index.vue (2)
30-54: LGTM - Reactivity properly maintained.The use of
storeToRefsensures thathasValidLicenseremains reactive and the UI updates when the license state changes.
78-80: LGTM - License info fetched on mount.Fetching license information when the component mounts ensures the UI has up-to-date license state.
packages/server/src/ee/routes/v1/boards.ts (1)
18-91: Now let me search the codebase more comprehensively to determine if the uniform licensing approach is universal across all route files:The identified licensing pattern is consistent architectural design, not an error.
The identical license requirements across all board operations (
["starter", "growth", "enterprise"]) match the consistent pattern observed in other route files (posts.ts,roadmaps.ts). Evenroles.tsapplies its more restrictive tier (["enterprise"]) uniformly across all operations. This indicates the codebase intentionally applies uniform licensing per resource rather than differentiating by HTTP method. The concern about copy-paste appears unfounded—this reflects an established architectural pattern.
| // @ts-expect-error | ||
| const roadmap = req.roadmap; |
There was a problem hiding this comment.
Remove @ts-expect-error and add runtime validation.
Using @ts-expect-error suppresses TypeScript's type checking, which can mask runtime errors if the middleware doesn't properly populate req.roadmap. This creates two problems:
- No compile-time guarantee that
req.roadmapexists - No runtime validation before accessing the property
Apply this diff to add proper runtime validation:
- // @ts-expect-error
- const roadmap = req.roadmap;
+ // @ts-expect-error - roadmap is set by middleware
+ const roadmap = req.roadmap;
+
+ if (!roadmap) {
+ return res.status(500).send({
+ message: "Roadmap not found in request context",
+ code: "ROADMAP_NOT_IN_CONTEXT"
+ });
+ }Additionally, once the Request interface is properly extended in express.d.ts (as suggested in the previous file review), remove the @ts-expect-error directive entirely. The combination of proper typing and runtime validation provides both compile-time and runtime safety.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // @ts-expect-error | |
| const roadmap = req.roadmap; | |
| // @ts-expect-error - roadmap is set by middleware | |
| const roadmap = req.roadmap; | |
| if (!roadmap) { | |
| return res.status(500).send({ | |
| message: "Roadmap not found in request context", | |
| code: "ROADMAP_NOT_IN_CONTEXT" | |
| }); | |
| } |
| import { withLicenseGuard } from "../../do-not-remove/middleware/licenseGuard"; | ||
|
|
||
| router.get<unknown, unknown, unknown, TFilterBoardRequestQuery>( | ||
| "/boards", | ||
| boards.filter, | ||
| // @ts-expect-error | ||
| withLicenseGuard(boards.filter, { | ||
| requiredPlan: ["starter", "growth", "enterprise"], | ||
| }), | ||
| ); | ||
| router.get<unknown, unknown, unknown, IGetBoardsRequestQuery>( | ||
| "/boards/get", | ||
| boards.get, | ||
| // @ts-expect-error | ||
| withLicenseGuard(boards.get, { | ||
| requiredPlan: ["starter", "growth", "enterprise"], | ||
| }), | ||
| ); | ||
| router.get<IGetBoardByUrlRequestParams>( | ||
| "/boards/:url", | ||
| boardExists, | ||
| boards.boardByUrl, | ||
| // @ts-expect-error | ||
| withLicenseGuard(boards.boardByUrl, { | ||
| requiredPlan: ["starter", "growth", "enterprise"], | ||
| }), | ||
| ); | ||
| router.get<ISearchBoardRequestParams>( | ||
| "/boards/search/:name", | ||
| // @ts-expect-error | ||
| authRequired, | ||
| boards.searchBoard, | ||
| // @ts-expect-error | ||
| withLicenseGuard(boards.searchBoard, { | ||
| requiredPlan: ["starter", "growth", "enterprise"], | ||
| }), | ||
| ); | ||
|
|
||
| router.post("/boards/check-slug", authRequired, boards.checkSlug); | ||
| router.post("/boards", authRequired, boards.create); | ||
| router.patch("/boards", authRequired, boardExists, boards.updateBoard); | ||
| router.post( | ||
| "/boards/check-slug", | ||
| authRequired, | ||
| withLicenseGuard(boards.checkSlug, { | ||
| requiredPlan: ["starter", "growth", "enterprise"], | ||
| }), | ||
| ); | ||
| router.post( | ||
| "/boards", | ||
| authRequired, | ||
| withLicenseGuard(boards.create, { | ||
| requiredPlan: ["starter", "growth", "enterprise"], | ||
| }), | ||
| ); | ||
| router.patch( | ||
| "/boards", | ||
| authRequired, | ||
| // @ts-expect-error | ||
| boardExists, | ||
| withLicenseGuard(boards.updateBoard, { | ||
| requiredPlan: ["starter", "growth", "enterprise"], | ||
| }), | ||
| ); | ||
|
|
||
| router.delete("/boards", authRequired, boardExists, boards.deleteById); | ||
| router.delete( | ||
| "/boards", | ||
| authRequired, | ||
| // @ts-expect-error | ||
| boardExists, | ||
| withLicenseGuard(boards.deleteById, { | ||
| requiredPlan: ["starter", "growth", "enterprise"], | ||
| }), | ||
| ); | ||
|
|
||
| // --- Start: Deprecated, will be removed in next major release --- | ||
| router.post("/boards/check-name", authRequired, boards.checkName); | ||
| router.post( | ||
| "/boards/check-name", | ||
| authRequired, | ||
| withLicenseGuard(boards.checkName, { | ||
| requiredPlan: ["starter", "growth", "enterprise"], | ||
| }), | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
Verify license guard implementation maintains proper Express middleware chain.
The withLicenseGuard wrapper is applied across all routes but the implementation isn't visible in this file. Ensure that:
- It properly maintains the Express middleware chain (req, res, next)
- It handles errors consistently with existing error response patterns
- It preserves request/response types through the wrapper
- It correctly short-circuits the chain when license validation fails
Run the following script to examine the license guard implementation:
🏁 Script executed:
#!/bin/bash
# Description: Review the withLicenseGuard implementation for proper middleware chaining
# Find and display the license guard implementation
echo "=== License Guard Implementation ==="
fd -t f licenseGuard.ts packages/server/src/ee --exec cat {}
echo -e "\n=== Check error response structure ==="
# Verify error response patterns used by license guard
rg -A 3 'error.*license' packages/server/src/errorResponse.jsonLength of output: 76
🏁 Script executed:
#!/bin/bash
# Search for licenseGuard file more broadly
echo "=== Searching for licenseGuard files ==="
fd -t f -i licenseguard
echo -e "\n=== Finding where withLicenseGuard is defined ==="
rg -n 'export.*withLicenseGuard|function withLicenseGuard|const withLicenseGuard' --type=ts
echo -e "\n=== Checking directory structure ==="
fd -t d 'do-not-remove' packages/server/src/ee
echo -e "\n=== Looking for license-related middleware ==="
rg -l 'licenseGuard|withLicenseGuard' packages/server/src --type=ts | head -10Length of output: 347
🏁 Script executed:
#!/bin/bash
# Discover actual repository structure
echo "=== Repository root structure ==="
ls -la
echo -e "\n=== Find boards.ts file ==="
find . -name 'boards.ts' -type f 2>/dev/null | head -5
echo -e "\n=== List directory structure for context ==="
find . -maxdepth 3 -type d -name 'server' -o -name 'packages' 2>/dev/null | head -10
echo -e "\n=== Search for any licenseGuard references ==="
find . -type f \( -name '*.ts' -o -name '*.js' \) -exec grep -l 'licenseGuard\|withLicenseGuard' {} \; 2>/dev/null | head -10Length of output: 2594
🏁 Script executed:
#!/bin/bash
# Find and examine the licenseGuard implementation
echo "=== License Guard Implementation ==="
cat ./packages/server/src/ee/do-not-remove/middleware/licenseGuard.ts
echo -e "\n=== Examine error handling in related middleware ==="
grep -r 'requiredPlan\|license.*check' ./packages/server/src/ee/routes -A 2 -B 2 --type=ts | head -30
echo -e "\n=== Check boards.ts imports for context ==="
head -20 ./packages/server/src/ee/routes/v1/boards.tsLength of output: 956
🏁 Script executed:
#!/bin/bash
# Broader search for licenseGuard file
echo "=== Search for any licenseGuard file ==="
find ./packages/server/src -name '*licenseGuard*' -type f 2>/dev/null
echo -e "\n=== List all files in ee/do-not-remove/middleware if it exists ==="
ls -la ./packages/server/src/ee/do-not-remove/middleware/ 2>/dev/null || echo "Directory not found"
echo -e "\n=== List ee directory structure ==="
find ./packages/server/src/ee -maxdepth 3 -type d 2>/dev/null
echo -e "\n=== Search for withLicenseGuard definition ==="
grep -r 'withLicenseGuard' ./packages/server/src --include='*.ts' --include='*.js' -B 2 -A 5 | head -40
echo -e "\n=== Check entire middleware directory ==="
find ./packages/server/src -path '*/middleware/*' -type f 2>/dev/null | head -20Length of output: 3721
🏁 Script executed:
#!/bin/bash
# Check what's in the do-not-remove directory
echo "=== Contents of do-not-remove directory ==="
find ./packages/server/src/ee/do-not-remove -type f 2>/dev/null
echo -e "\n=== Full tree of do-not-remove ==="
find ./packages/server/src/ee/do-not-remove -type d 2>/dev/null
echo -e "\n=== Search for any license-related code in codebase ==="
grep -r 'export.*withLicenseGuard\|function withLicenseGuard' ./packages/server/src --include='*.ts' --include='*.js' 2>/dev/null
echo -e "\n=== Check if there's a .gitignore that might hide the file ==="
cat ./packages/server/src/ee/do-not-remove/.gitignore 2>/dev/null || echo "No .gitignore found"Length of output: 259
The import path is broken—withLicenseGuard file does not exist.
The file packages/server/src/ee/do-not-remove/middleware/licenseGuard.ts is missing entirely. The do-not-remove directory exists but is empty, with no middleware subdirectory. This import will fail at runtime and prevent the entire routes module from loading.
Create the missing middleware file with the proper license guard implementation, or update the import path if the middleware exists elsewhere.
| // @ts-expect-error | ||
| withLicenseGuard(boards.filter, { | ||
| requiredPlan: ["starter", "growth", "enterprise"], | ||
| }), |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Multiple type suppressions indicate systematic type incompatibility.
The repeated @ts-expect-error markers before withLicenseGuard across all routes indicate that the wrapper's type signature is incompatible with Express's expected RequestHandler type. This reduces type safety and IDE support.
Consider fixing the type signature of withLicenseGuard to properly extend Express middleware types, or create a properly typed wrapper function that satisfies TypeScript's type checker.
Example approach:
// In licenseGuard.ts, ensure proper typing:
import type { RequestHandler } from 'express';
export function withLicenseGuard<P = any, ResBody = any, ReqBody = any, ReqQuery = any>(
handler: RequestHandler<P, ResBody, ReqBody, ReqQuery>,
options: LicenseGuardOptions
): RequestHandler<P, ResBody, ReqBody, ReqQuery> {
// implementation
}Also applies to: 27-30, 35-38, 44-47, 53-55, 60-62, 69-71, 79-81, 88-90
| TEmailVerification, | ||
| TResetPassword, | ||
| } from "@logchimp/types"; | ||
| import type { TEmailVerification, TResetPassword } from "@logchimp/types"; |
There was a problem hiding this comment.
Add proper type definition for req.roadmap instead of removing it from context.
The removal of roadmap from ExpressRequestContext.ctx is causing type safety issues throughout the codebase. The second file in this PR uses @ts-expect-error to suppress TypeScript errors when accessing req.roadmap, which defeats the purpose of type safety.
Instead of removing the property entirely, extend the Express Request interface to include the roadmap property:
import type { Request } from "express";
-import type { TEmailVerification, TResetPassword } from "@logchimp/types";
+import type {
+ IRoadmapPrivate,
+ TEmailVerification,
+ TResetPassword
+} from "@logchimp/types";
+
+declare global {
+ namespace Express {
+ interface Request {
+ roadmap?: IRoadmapPrivate;
+ }
+ }
+}This approach:
- Maintains type safety across the codebase
- Eliminates the need for
@ts-expect-errordirectives - Follows Express's recommended pattern for custom properties
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import type { TEmailVerification, TResetPassword } from "@logchimp/types"; | |
| import type { Request } from "express"; | |
| import type { | |
| IRoadmapPrivate, | |
| TEmailVerification, | |
| TResetPassword | |
| } from "@logchimp/types"; | |
| declare global { | |
| namespace Express { | |
| interface Request { | |
| roadmap?: IRoadmapPrivate; | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In packages/server/src/express.d.ts around line 2, restore a typed roadmap
property on Express Request by declaring a global augmentation: import (or
create) the appropriate Roadmap type from your types (or module where it's
defined) and add a declaration like "declare global { namespace Express {
interface Request { roadmap: RoadmapType } } }" so req.roadmap is properly typed
across the codebase; if no centralized Roadmap type exists, define a small
interface that matches the shape used and use that here to avoid
@ts-expect-error hacks.
| <UpgradeTooltip :has-valid-license="hasValidLicense"> | ||
| <Button | ||
| type="primary" | ||
| :disabled="createBoardPermissionDisabled" | ||
| :loading="createBoardButtonLoading" | ||
| @click="createBoardHandler" | ||
| > | ||
| Create board | ||
| <LicenseCrown v-if="!hasValidLicense" /> | ||
| </Button> | ||
| </UpgradeTooltip> |
There was a problem hiding this comment.
Consider disabling the button when license is invalid.
The Create Board button remains clickable (if the user has permission) even when hasValidLicense is false. This allows users to trigger createBoardHandler, which will likely fail on the backend, resulting in a confusing experience.
Apply this diff to disable the button when the license is invalid:
<UpgradeTooltip :has-valid-license="hasValidLicense">
<Button
type="primary"
- :disabled="createBoardPermissionDisabled"
+ :disabled="createBoardPermissionDisabled || !hasValidLicense"
:loading="createBoardButtonLoading"
@click="createBoardHandler"
>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <UpgradeTooltip :has-valid-license="hasValidLicense"> | |
| <Button | |
| type="primary" | |
| :disabled="createBoardPermissionDisabled" | |
| :loading="createBoardButtonLoading" | |
| @click="createBoardHandler" | |
| > | |
| Create board | |
| <LicenseCrown v-if="!hasValidLicense" /> | |
| </Button> | |
| </UpgradeTooltip> | |
| <UpgradeTooltip :has-valid-license="hasValidLicense"> | |
| <Button | |
| type="primary" | |
| :disabled="createBoardPermissionDisabled || !hasValidLicense" | |
| :loading="createBoardButtonLoading" | |
| @click="createBoardHandler" | |
| > | |
| Create board | |
| <LicenseCrown v-if="!hasValidLicense" /> | |
| </Button> | |
| </UpgradeTooltip> |
🤖 Prompt for AI Agents
In packages/theme/src/ee/pages/dashboard/boards/Index.vue around lines 9 to 19,
the Create Board button is still clickable when hasValidLicense is false; update
the button's disabled binding to include the license check (e.g., disable when
createBoardPermissionDisabled OR not hasValidLicense) so clicks cannot trigger
createBoardHandler for invalid licenses, and keep the existing loading and
tooltip behavior intact.
| } catch (e) { | ||
| // @ts-expect-error | ||
| const isInvalid = e.response?.data?.code === "LICENSE_VALIDATION_FAILED"; | ||
| hasValidLicense.value = !isInvalid; | ||
| } |
There was a problem hiding this comment.
Fix error handling logic for license validation.
The current error handling incorrectly sets hasValidLicense to true when the API call fails for reasons other than LICENSE_VALIDATION_FAILED (e.g., network errors, server errors, timeouts). This could grant access to license-gated features when the license status is unknown or the API is unavailable.
Apply this diff to fix the error handling:
} catch (e) {
// @ts-expect-error
const isInvalid = e.response?.data?.code === "LICENSE_VALIDATION_FAILED";
- hasValidLicense.value = !isInvalid;
+ if (isInvalid) {
+ hasValidLicense.value = false;
+ }
+ // For other errors (network, server errors), keep the existing state
+ // or consider logging the error
}Alternatively, if you want to be more conservative and block access on any error:
} catch (e) {
// @ts-expect-error
const isInvalid = e.response?.data?.code === "LICENSE_VALIDATION_FAILED";
- hasValidLicense.value = !isInvalid;
+ hasValidLicense.value = false;
+ // Optional: log non-validation errors for debugging
+ if (!isInvalid) {
+ console.error("License check failed:", e);
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } catch (e) { | |
| // @ts-expect-error | |
| const isInvalid = e.response?.data?.code === "LICENSE_VALIDATION_FAILED"; | |
| hasValidLicense.value = !isInvalid; | |
| } | |
| } catch (e) { | |
| // @ts-expect-error | |
| const isInvalid = e.response?.data?.code === "LICENSE_VALIDATION_FAILED"; | |
| hasValidLicense.value = false; | |
| // Optional: log non-validation errors for debugging | |
| if (!isInvalid) { | |
| console.error("License check failed:", e); | |
| } | |
| } |
| } catch (e) { | |
| // @ts-expect-error | |
| const isInvalid = e.response?.data?.code === "LICENSE_VALIDATION_FAILED"; | |
| hasValidLicense.value = !isInvalid; | |
| } | |
| } catch (e) { | |
| // @ts-expect-error | |
| const isInvalid = e.response?.data?.code === "LICENSE_VALIDATION_FAILED"; | |
| if (isInvalid) { | |
| hasValidLicense.value = false; | |
| } | |
| // For other errors (network, server errors), keep the existing state | |
| // or consider logging the error | |
| } |
🤖 Prompt for AI Agents
In packages/theme/src/ee/store/settings.ts around lines 16 to 20, the catch
block currently flips hasValidLicense to true for any error that isn't
LICENSE_VALIDATION_FAILED; change the logic so that any caught error results in
hasValidLicense.value being set to false (i.e., deny access on API errors), and
optionally log the error for debugging; do not set hasValidLicense to true
unless the API call explicitly indicates a valid license in the success path.
Summary by CodeRabbit