-
Notifications
You must be signed in to change notification settings - Fork 714
Stage #9276
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Stage #9276
Conversation
The postinstall.js script is plain JavaScript and doesn't require ts-node. This fixes the Docker build error where ts-node was not available to the node user.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
25 issues found across 47 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/mcp-server/src/lib/tools/expenses.ts">
<violation number="1" location="packages/mcp-server/src/lib/tools/expenses.ts:181">
P2: The `currency` field uses `z.string()` instead of the imported `CurrenciesEnum`, allowing invalid currency values. This is inconsistent with `get_expenses` which validates currency against the enum. Use `CurrenciesEnum` for consistent validation.</violation>
</file>
<file name="packages/mcp-server/src/lib/tools/daily-plan.ts">
<violation number="1" location="packages/mcp-server/src/lib/tools/daily-plan.ts:326">
P2: Inconsistent status validation: Using `z.string()` for the status field while `DailyPlanStatusEnum` is imported and used for query parameters elsewhere in this file. Consider using `DailyPlanStatusEnum` here for consistent validation.</violation>
<violation number="2" location="packages/mcp-server/src/lib/tools/daily-plan.ts:374">
P2: Using `z.record(z.string(), z.any())` removes type safety for the update payload. The `z.any()` accepts any value type without validation, which could allow invalid data to be passed to the API. Consider defining a more specific schema or at minimum using `z.unknown()` instead of `z.any()` to require explicit type handling.</violation>
</file>
<file name="packages/mcp-server/src/lib/tools/organization-contact.ts">
<violation number="1" location="packages/mcp-server/src/lib/tools/organization-contact.ts:253">
P2: Missing UUID validation for `organizationId`. Other tools in this file validate organizationId as UUID (e.g., `get_organization_contacts` uses `z.string().uuid()`). Consider adding `.uuid()` for consistency and proper validation.</violation>
<violation number="2" location="packages/mcp-server/src/lib/tools/organization-contact.ts:418">
P2: Missing UUID validation for `id`. Other tools validate ID fields as UUID. Consider using `z.string().uuid()` for consistency.</violation>
<violation number="3" location="packages/mcp-server/src/lib/tools/organization-contact.ts:420">
P2: Missing UUID validation for `organizationId` in bulk update. Should use `z.string().uuid()` for consistency with other tools.</violation>
<violation number="4" location="packages/mcp-server/src/lib/tools/organization-contact.ts:421">
P2: Missing UUID validation for `tenantId` in bulk update. Should use `z.string().uuid()` for consistency with other tools.</violation>
</file>
<file name="packages/mcp-server/src/lib/common/auth-types.ts">
<violation number="1" location="packages/mcp-server/src/lib/common/auth-types.ts:154">
P2: Values interpolated into the WWW-Authenticate header should be escaped to prevent header injection. Double quotes and backslashes in error values would create malformed headers per RFC 7230.</violation>
</file>
<file name="packages/mcp-server/src/lib/tools/candidates.ts">
<violation number="1" location="packages/mcp-server/src/lib/tools/candidates.ts:171">
P2: The `userId` field lost its UUID validation. The original schema enforced `z.string().uuid()` format, but this now only validates it as a string. Consider using `z.string().uuid()` to maintain input validation.</violation>
<violation number="2" location="packages/mcp-server/src/lib/tools/candidates.ts:208">
P2: Using `z.record(z.string(), z.any())` removes all field-level validation. The original `CandidateSchema.partial()` validated field types, enums, ranges (e.g., rating 0-5), and UUIDs. Consider maintaining stricter validation for data integrity.</violation>
</file>
<file name="packages/mcp-server/src/lib/common/security-logger.ts">
<violation number="1" location="packages/mcp-server/src/lib/common/security-logger.ts:89">
P2: The static `events` array is populated in `logSecurityEvent()` but never exposed via any getter method. The existing `getRecentLogs()`, `getLogsByEvent()`, and `getLogsByUser()` methods only return instance logs (`this.logs`), not static events. Consider adding a static getter method like `getStaticEvents()` or removing the static storage if unused.</violation>
<violation number="2" location="packages/mcp-server/src/lib/common/security-logger.ts:115">
P1: The `success` field is incorrectly derived from severity level. A security event's success/failure status is independent of its severity. For example, `ACCESS_GRANTED` with 'medium' severity should still be `success: true`. Consider passing `success` as a parameter or deriving it from the event type.</violation>
</file>
<file name="packages/mcp-server/src/lib/tools/employees.ts">
<violation number="1" location="packages/mcp-server/src/lib/tools/employees.ts:1">
P1: Adding `@ts-nocheck` disables TypeScript type checking for the entire file, which can hide real bugs and type errors. Consider fixing the underlying type issues instead of suppressing all checks.</violation>
<violation number="2" location="packages/mcp-server/src/lib/tools/employees.ts:413">
P2: The schema now only validates `userId` as a generic string (not UUID) and `.passthrough()` allows any additional fields without validation. This is less strict than the previous `EmployeeSchema.partial().required({ userId: true })`. Consider validating `userId` as UUID with `z.string().uuid()` for consistency with other ID fields in the file.</violation>
</file>
<file name="packages/mcp-server/src/lib/tools/goals.ts">
<violation number="1" location="packages/mcp-server/src/lib/tools/goals.ts:171">
P2: The `level` field should use `GoalLevelEnum` instead of `z.string()` for consistency. `GoalLevelEnum` is already imported and used elsewhere in this file for filtering. Using the enum provides better validation and documents valid values ('ORGANIZATION', 'TEAM', 'EMPLOYEE') in the schema.</violation>
</file>
<file name="packages/mcp-server/src/lib/tools/projects.ts">
<violation number="1" location="packages/mcp-server/src/lib/tools/projects.ts:339">
P2: Using `z.record(z.string(), z.any())` removes all type safety for project update data. This allows arbitrary data to be passed through without validation, which could lead to data integrity issues or security vulnerabilities. Consider keeping a typed schema or at least validating expected fields.</violation>
</file>
<file name="packages/mcp-server/src/lib/tools/comments.ts">
<violation number="1" location="packages/mcp-server/src/lib/tools/comments.ts:1">
P1: Adding `// @ts-nocheck` disables TypeScript type checking for the entire file. This removes valuable compile-time safety and can hide bugs. Consider addressing the underlying type issues instead of suppressing all type checking.</violation>
<violation number="2" location="packages/mcp-server/src/lib/tools/comments.ts:151">
P2: The `entity` field uses `z.string()` but should use `CommentableTypeEnum` for consistency with other tools in this file (e.g., `reply_to_comment`, `get_comments_by_entity`). This ensures only valid entity types are accepted.</violation>
<violation number="3" location="packages/mcp-server/src/lib/tools/comments.ts:152">
P2: The `entityId` field is missing `.uuid()` validation. Other tools in this file consistently use `z.string().uuid()` for entity IDs. This ensures proper ID format validation before making API calls.</violation>
</file>
<file name="packages/mcp-server/src/lib/tools/incomes.ts">
<violation number="1" location="packages/mcp-server/src/lib/tools/incomes.ts:273">
P1: Using `z.record(z.string(), z.any())` is overly permissive and removes all field-level validation. This could allow invalid data types (e.g., string instead of number for `amount`) or unexpected fields to be sent to the API. Consider defining explicit allowed fields with their types, similar to the `create_income` schema.</violation>
</file>
<file name="packages/mcp-server/src/lib/tools/employee-awards.ts">
<violation number="1" location="packages/mcp-server/src/lib/tools/employee-awards.ts:105">
P2: `employeeId` is missing `.uuid()` validation, which is inconsistent with other employeeId fields in this file that validate UUID format. This could allow invalid employee IDs.</violation>
</file>
<file name="packages/mcp-server/src/lib/tools/time-off.ts">
<violation number="1" location="packages/mcp-server/src/lib/tools/time-off.ts:190">
P2: `employeeId` should use `z.string().uuid()` to maintain proper UUID validation, consistent with other ID fields in this file (e.g., the `id` field in `update_time_off_request` uses `.uuid()`).</violation>
<violation number="2" location="packages/mcp-server/src/lib/tools/time-off.ts:191">
P2: `policyId` should use `z.string().uuid()` to maintain proper UUID validation, consistent with other ID fields in this file.</violation>
</file>
<file name="packages/mcp-server/src/lib/tools/payments.ts">
<violation number="1" location="packages/mcp-server/src/lib/tools/payments.ts:1">
P1: Avoid using `// @ts-nocheck` as it disables all TypeScript type checking for the file, potentially hiding real type errors and making refactoring dangerous. If there are specific type issues, address them individually with `// @ts-expect-error` on specific lines with explanatory comments, or fix the underlying type issues.</violation>
<violation number="2" location="packages/mcp-server/src/lib/tools/payments.ts:206">
P2: `CurrenciesEnum` is imported but not used for the currency field. Using `z.string()` allows any string value, while `CurrenciesEnum` would enforce valid currency codes (USD, EUR, GBP, JPY, CAD, AUD, CHF, CNY). Consider using `CurrenciesEnum` for consistency with other tools in this file.</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
| expense_data: z | ||
| .object({ | ||
| amount: z.number().describe('The amount (required)'), | ||
| currency: z.string().describe('The currency (required)') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2: The currency field uses z.string() instead of the imported CurrenciesEnum, allowing invalid currency values. This is inconsistent with get_expenses which validates currency against the enum. Use CurrenciesEnum for consistent validation.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/mcp-server/src/lib/tools/expenses.ts, line 181:
<comment>The `currency` field uses `z.string()` instead of the imported `CurrenciesEnum`, allowing invalid currency values. This is inconsistent with `get_expenses` which validates currency against the enum. Use `CurrenciesEnum` for consistent validation.</comment>
<file context>
@@ -165,15 +170,17 @@ export const registerExpenseTools = (server: McpServer) => {
+ expense_data: z
+ .object({
+ amount: z.number().describe('The amount (required)'),
+ currency: z.string().describe('The currency (required)')
})
+ .passthrough()
</file context>
| currency: z.string().describe('The currency (required)') | |
| currency: CurrenciesEnum.describe('The currency (required)') |
| .object({ | ||
| date: z.string().describe('The date (required)'), | ||
| workTimePlanned: z.number().describe('Work time planned (required)'), | ||
| status: z.string().describe('The status (required)') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2: Inconsistent status validation: Using z.string() for the status field while DailyPlanStatusEnum is imported and used for query parameters elsewhere in this file. Consider using DailyPlanStatusEnum here for consistent validation.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/mcp-server/src/lib/tools/daily-plan.ts, line 326:
<comment>Inconsistent status validation: Using `z.string()` for the status field while `DailyPlanStatusEnum` is imported and used for query parameters elsewhere in this file. Consider using `DailyPlanStatusEnum` here for consistent validation.</comment>
<file context>
@@ -306,16 +314,18 @@ export const registerDailyPlanTools = (server: McpServer) => {
+ .object({
+ date: z.string().describe('The date (required)'),
+ workTimePlanned: z.number().describe('Work time planned (required)'),
+ status: z.string().describe('The status (required)')
})
+ .passthrough()
</file context>
| status: z.string().describe('The status (required)') | |
| status: DailyPlanStatusEnum.describe('The status (required)') |
| { | ||
| id: z.string().uuid().describe('The daily plan ID'), | ||
| plan_data: DailyPlanSchema.partial().describe('The fields to update on the daily plan') | ||
| plan_data: z.record(z.string(), z.any()).describe('The fields to update on the daily plan') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2: Using z.record(z.string(), z.any()) removes type safety for the update payload. The z.any() accepts any value type without validation, which could allow invalid data to be passed to the API. Consider defining a more specific schema or at minimum using z.unknown() instead of z.any() to require explicit type handling.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/mcp-server/src/lib/tools/daily-plan.ts, line 374:
<comment>Using `z.record(z.string(), z.any())` removes type safety for the update payload. The `z.any()` accepts any value type without validation, which could allow invalid data to be passed to the API. Consider defining a more specific schema or at minimum using `z.unknown()` instead of `z.any()` to require explicit type handling.</comment>
<file context>
@@ -355,12 +365,13 @@ export const registerDailyPlanTools = (server: McpServer) => {
{
id: z.string().uuid().describe('The daily plan ID'),
- plan_data: DailyPlanSchema.partial().describe('The fields to update on the daily plan')
+ plan_data: z.record(z.string(), z.any()).describe('The fields to update on the daily plan')
},
async ({ id, plan_data }) => {
</file context>
| id: z.string().describe('The ID (required)'), | ||
| name: z.string().describe('The name (required)'), | ||
| organizationId: z.string().describe('The organization ID (required)'), | ||
| tenantId: z.string().describe('The tenant ID (required)') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2: Missing UUID validation for tenantId in bulk update. Should use z.string().uuid() for consistency with other tools.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/mcp-server/src/lib/tools/organization-contact.ts, line 421:
<comment>Missing UUID validation for `tenantId` in bulk update. Should use `z.string().uuid()` for consistency with other tools.</comment>
<file context>
@@ -395,18 +406,21 @@ export const registerOrganizationContactTools = (server: McpServer) => {
+ id: z.string().describe('The ID (required)'),
+ name: z.string().describe('The name (required)'),
+ organizationId: z.string().describe('The organization ID (required)'),
+ tenantId: z.string().describe('The tenant ID (required)')
+ })
+ .passthrough()
</file context>
| .object({ | ||
| id: z.string().describe('The ID (required)'), | ||
| name: z.string().describe('The name (required)'), | ||
| organizationId: z.string().describe('The organization ID (required)'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2: Missing UUID validation for organizationId in bulk update. Should use z.string().uuid() for consistency with other tools.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/mcp-server/src/lib/tools/organization-contact.ts, line 420:
<comment>Missing UUID validation for `organizationId` in bulk update. Should use `z.string().uuid()` for consistency with other tools.</comment>
<file context>
@@ -395,18 +406,21 @@ export const registerOrganizationContactTools = (server: McpServer) => {
+ .object({
+ id: z.string().describe('The ID (required)'),
+ name: z.string().describe('The name (required)'),
+ organizationId: z.string().describe('The organization ID (required)'),
+ tenantId: z.string().describe('The tenant ID (required)')
+ })
</file context>
| .object({ | ||
| name: z.string().describe('The award name (required)'), | ||
| year: z.number().describe('The year (required)'), | ||
| employeeId: z.string().describe('The employee ID (required)') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2: employeeId is missing .uuid() validation, which is inconsistent with other employeeId fields in this file that validate UUID format. This could allow invalid employee IDs.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/mcp-server/src/lib/tools/employee-awards.ts, line 105:
<comment>`employeeId` is missing `.uuid()` validation, which is inconsistent with other employeeId fields in this file that validate UUID format. This could allow invalid employee IDs.</comment>
<file context>
@@ -89,16 +93,18 @@ export const registerEmployeeAwardTools = (server: McpServer) => {
+ .object({
+ name: z.string().describe('The award name (required)'),
+ year: z.number().describe('The year (required)'),
+ employeeId: z.string().describe('The employee ID (required)')
})
+ .passthrough()
</file context>
| employeeId: z.string().describe('The employee ID (required)') | |
| employeeId: z.string().uuid().describe('The employee ID (required)') |
| startDate: z.string().describe('The start date (required)'), | ||
| endDate: z.string().describe('The end date (required)'), | ||
| employeeId: z.string().describe('The employee ID (required)'), | ||
| policyId: z.string().describe('The policy ID (required)') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2: policyId should use z.string().uuid() to maintain proper UUID validation, consistent with other ID fields in this file.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/mcp-server/src/lib/tools/time-off.ts, line 191:
<comment>`policyId` should use `z.string().uuid()` to maintain proper UUID validation, consistent with other ID fields in this file.</comment>
<file context>
@@ -173,17 +178,19 @@ export const registerTimeOffTools = (server: McpServer) => {
+ startDate: z.string().describe('The start date (required)'),
+ endDate: z.string().describe('The end date (required)'),
+ employeeId: z.string().describe('The employee ID (required)'),
+ policyId: z.string().describe('The policy ID (required)')
})
+ .passthrough()
</file context>
| policyId: z.string().describe('The policy ID (required)') | |
| policyId: z.string().uuid().describe('The policy ID (required)') |
| .object({ | ||
| startDate: z.string().describe('The start date (required)'), | ||
| endDate: z.string().describe('The end date (required)'), | ||
| employeeId: z.string().describe('The employee ID (required)'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2: employeeId should use z.string().uuid() to maintain proper UUID validation, consistent with other ID fields in this file (e.g., the id field in update_time_off_request uses .uuid()).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/mcp-server/src/lib/tools/time-off.ts, line 190:
<comment>`employeeId` should use `z.string().uuid()` to maintain proper UUID validation, consistent with other ID fields in this file (e.g., the `id` field in `update_time_off_request` uses `.uuid()`).</comment>
<file context>
@@ -173,17 +178,19 @@ export const registerTimeOffTools = (server: McpServer) => {
+ .object({
+ startDate: z.string().describe('The start date (required)'),
+ endDate: z.string().describe('The end date (required)'),
+ employeeId: z.string().describe('The employee ID (required)'),
+ policyId: z.string().describe('The policy ID (required)')
})
</file context>
| employeeId: z.string().describe('The employee ID (required)'), | |
| employeeId: z.string().uuid().describe('The employee ID (required)'), |
| payment_data: z | ||
| .object({ | ||
| amount: z.number().describe('The amount (required)'), | ||
| currency: z.string().describe('The currency (required)') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2: CurrenciesEnum is imported but not used for the currency field. Using z.string() allows any string value, while CurrenciesEnum would enforce valid currency codes (USD, EUR, GBP, JPY, CAD, AUD, CHF, CNY). Consider using CurrenciesEnum for consistency with other tools in this file.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/mcp-server/src/lib/tools/payments.ts, line 206:
<comment>`CurrenciesEnum` is imported but not used for the currency field. Using `z.string()` allows any string value, while `CurrenciesEnum` would enforce valid currency codes (USD, EUR, GBP, JPY, CAD, AUD, CHF, CNY). Consider using `CurrenciesEnum` for consistency with other tools in this file.</comment>
<file context>
@@ -190,15 +195,17 @@ export const registerPaymentTools = (server: McpServer) => {
+ payment_data: z
+ .object({
+ amount: z.number().describe('The amount (required)'),
+ currency: z.string().describe('The currency (required)')
})
+ .passthrough()
</file context>
| currency: z.string().describe('The currency (required)') | |
| currency: CurrenciesEnum.describe('The currency (required)') |
| @@ -1,11 +1,13 @@ | |||
| // @ts-nocheck | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P1: Avoid using // @ts-nocheck as it disables all TypeScript type checking for the file, potentially hiding real type errors and making refactoring dangerous. If there are specific type issues, address them individually with // @ts-expect-error on specific lines with explanatory comments, or fix the underlying type issues.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/mcp-server/src/lib/tools/payments.ts, line 1:
<comment>Avoid using `// @ts-nocheck` as it disables all TypeScript type checking for the file, potentially hiding real type errors and making refactoring dangerous. If there are specific type issues, address them individually with `// @ts-expect-error` on specific lines with explanatory comments, or fix the underlying type issues.</comment>
<file context>
@@ -1,11 +1,13 @@
+// @ts-nocheck
import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js';
import { Logger } from '@nestjs/common';
</file context>
Greptile SummaryThis PR successfully decouples the MCP server from Key Changes:
Issues Found:
Confidence Score: 3/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Client as LLM Client
participant HTTP as HTTP Transport
participant Auth as Authorization
participant MCP as MCP Server
participant Tools as Tool Helper
participant API as Gauzy API
Note over HTTP,MCP: Decoupled from gauzy auth package
Client->>HTTP: MCP Request
HTTP->>Auth: Check authorization
Note over Auth: Uses local auth utilities
alt Auth Enabled
Auth-->>HTTP: Not implemented
HTTP-->>Client: Unauthorized
else Auth Disabled
HTTP->>MCP: Forward request
MCP->>Tools: Register tool
Note over Tools: Bypass Zod inference
Tools->>Tools: Validate schema
Tools->>API: Execute call
API-->>Tools: Response data
Tools-->>MCP: Tool result
MCP-->>HTTP: Success
HTTP->>HTTP: Add headers
HTTP-->>Client: JSON Response
end
Note over MCP,API: All tools use registerTool
Note over Tools: Type checking relaxed
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
47 files reviewed, 4 comments
| @@ -1,15 +1,11 @@ | |||
| // @ts-nocheck | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: // @ts-nocheck disables all type checking for this file. This hides type errors that should be fixed.
| // @ts-nocheck | |
| // Remove @ts-nocheck and fix type errors properly |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/mcp-server/src/lib/tools/tasks.ts
Line: 1:1
Comment:
**style:** `// @ts-nocheck` disables all type checking for this file. This hides type errors that should be fixed.
```suggestion
// Remove @ts-nocheck and fix type errors properly
```
How can I resolve this? If you propose a fix, please make it concise.| // TODO: Implement actual JWT validation when authorization is enabled | ||
| // For now, return invalid if authorization is enabled but no implementation | ||
| return { | ||
| valid: false, | ||
| error: 'Token validation not implemented' | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: Token validation returns valid: false when authorization is enabled, effectively blocking all requests. This creates a security issue where the feature appears enabled but isn't functional. Is authorization intended to be non-functional in this PR, or should there be basic JWT validation implemented?
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/mcp-server/src/lib/common/auth-types.ts
Line: 117:122
Comment:
**logic:** Token validation returns `valid: false` when authorization is enabled, effectively blocking all requests. This creates a security issue where the feature appears enabled but isn't functional. Is authorization intended to be non-functional in this PR, or should there be basic JWT validation implemented?
How can I resolve this? If you propose a fix, please make it concise.| import { sanitizeErrorMessage, sanitizeForLogging } from '../common/error-utils'; | ||
| import type { SessionData } from '../session/session-store'; | ||
|
|
||
| import { registerTool, registerNoArgsTool } from './tool-helper'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: registerNoArgsTool is imported but never used in this file. All tools use registerTool with empty schema objects.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/mcp-server/src/lib/tools/auth.ts
Line: 11:11
Comment:
**style:** `registerNoArgsTool` is imported but never used in this file. All tools use `registerTool` with empty schema objects.
How can I resolve this? If you propose a fix, please make it concise.| .required({ | ||
| title: true | ||
| }) | ||
| task_data: z.object({ title: z.string().describe('The title (required)') }).passthrough() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: .passthrough() allows any additional properties, losing type safety. The previous TaskSchema.partial() provided validation for all task fields.
| task_data: z.object({ title: z.string().describe('The title (required)') }).passthrough() | |
| task_data: z.object({ | |
| title: z.string().describe('The title (required)'), | |
| // Add other task fields with proper validation | |
| }).describe('The data for creating the task') |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/mcp-server/src/lib/tools/tasks.ts
Line: 367:367
Comment:
**style:** `.passthrough()` allows any additional properties, losing type safety. The previous `TaskSchema.partial()` provided validation for all task fields.
```suggestion
task_data: z.object({
title: z.string().describe('The title (required)'),
// Add other task fields with proper validation
}).describe('The data for creating the task')
```
How can I resolve this? If you propose a fix, please make it concise.
PR
Please note: we will close your PR without comment if you do not check the boxes above and provide ALL requested information.
Summary by cubic
Decoupled MCP Server from @gauzy/auth and added lightweight local auth, logging, and schema utilities to fix TypeScript OOMs and speed up builds. Also fixed Docker postinstall error and aligned package outputs to dist for reliable runtime.
Refactors
Bug Fixes
Written for commit c1ae5c2. Summary will update automatically on new commits.