[VPAT-55] chore(security): implement input validation across authentication and workspace forms #8528
Conversation
… workspace forms
- Add OWASP-compliant autocomplete attributes to all auth input fields
- Create centralized validation utilities blocking injection-risk characters
- Apply validation to names, display names, workspace names, and slugs
- Block special characters: < > ' " % # { } [ ] * ^ !
- Secure sensitive input fields across admin, web, and space apps
📝 WalkthroughWalkthroughAdds a new validation utilities module in Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/core/components/workspace/create-workspace-form.tsx (1)
161-177: Slug validation not integrated with form submission.
validateSlugis called inonChangeto setinvalidSlugstate, but it's not added as avalidaterule inrules. The form'sisValidstate (which controls button disable at line 238) doesn't considerinvalidSlug, allowing submission with invalid characters.🔧 Proposed fix — add validate rule
rules={{ required: t("common.errors.required"), + validate: validateSlug, maxLength: { value: 48, message: t("workspace_creation.errors.validation.url_length"), }, }}With this fix, you can also remove the
invalidSlugstate and the manual onChange validation since react-hook-form will handle it:onChange={(e) => { - const validation = validateSlug(e.target.value); - if (validation === true) setInvalidSlug(false); - else setInvalidSlug(true); onChange(e.target.value.toLowerCase()); }}And update the error display to use
errors.sluginstead of the separateinvalidSlugstate.
🤖 Fix all issues with AI agents
In @apps/admin/core/components/instance/setup-form.tsx:
- Around line 238-244: The company name field is marked required in the UI but
validateCompanyName is being called with required: false; update the onChange
handler so validateCompanyName is called with required: true (instead of false)
so the required validation is enforced before calling handleFormChange for
"company_name"; ensure the rest of the handler behavior remains the same and
that maxLength/empty-string allowance logic still matches your intended UX.
In @apps/web/core/components/onboarding/create-workspace.tsx:
- Around line 196-200: The slug validation is using the raw input value instead
of the transformed/displayed slug; update the onChange handler in the
create-workspace component so it computes the transformedSlug (e.g., const
transformed = e.target.value.toLowerCase()) and run validateSlug(transformed)
before calling setInvalidSlug and onChange; specifically adjust the handler
around validateSlug, setInvalidSlug, and onChange to operate on the transformed
value rather than e.target.value.
In @apps/web/core/components/onboarding/steps/workspace/create.tsx:
- Around line 214-218: The handler is validating the raw input instead of the
displayed/transformed slug; compute the transformed slug first (same logic used
for display: trim, toLowerCase, replace spaces with hyphens), then call
validateSlug on that transformed value, setInvalidSlug based on that result, and
pass the transformed slug into onChange instead of e.target.value so validation
matches the shown value (refer to validateSlug, setInvalidSlug, and the onChange
handler).
In @apps/web/core/components/profile/form.tsx:
- Around line 285-287: The last_name field label is presented as optional but
its rules use validatePersonName which rejects empty values; either make the
field truly optional by adding a validator that permits empty input (e.g.,
create validateOptionalPersonName in validation.ts that returns true for
empty/whitespace and otherwise enforces the same length/regex checks as
validatePersonName, then swap validatePersonName for validateOptionalPersonName
in the rules) or make the field required in the UI (update the label to indicate
required and add a required rule alongside validatePersonName). Ensure you
reference and update validatePersonName/validateOptionalPersonName and the
last_name field's rules/label accordingly.
In @packages/utils/src/validation.ts:
- Around line 187-190: Remove the dead exported function hasInjectionRiskChars
(and its internal injectionRiskPattern) from packages/utils/src/validation.ts;
delete its export and any associated declarations, and update any barrel exports
or public API lists (e.g., index files) that re-export hasInjectionRiskChars so
the codebase no longer references an unused symbol.
🧹 Nitpick comments (6)
apps/web/core/components/onboarding/steps/profile/root.tsx (1)
203-209: Redundant validation rules.The
validatePersonNamefunction already checks for empty values and max length (50 chars), making therequiredandmaxLengthrules redundant. This also causes inconsistent error messages —validatePersonNamereturns "Name must be 50 characters or less" whilemaxLengthreturns "Name must be within 50 characters."Consider removing the redundant rules:
♻️ Suggested simplification
rules={{ - required: "Name is required", validate: validatePersonName, - maxLength: { - value: 50, - message: "Name must be within 50 characters.", - }, }}apps/web/core/components/workspace/create-workspace-form.tsx (1)
122-129: Minor redundancy in validation rules.
validateWorkspaceNamealready enforces max 80 characters, making themaxLengthrule redundant. Consider consolidating to avoid dual validation:♻️ Suggested simplification
rules={{ required: t("common.errors.required"), validate: (value) => validateWorkspaceName(value, true), - maxLength: { - value: 80, - message: t("workspace_creation.errors.validation.name_length"), - }, }}apps/web/core/components/onboarding/profile-setup.tsx (1)
298-305: Redundant validation rules.Similar to other files in this PR, the
requiredandmaxLengthrules duplicate checks already performed byvalidatePersonName. Consider removing the redundant rules for consistency and to avoid conflicting error messages.♻️ Suggested simplification for both name fields
rules={{ - required: "First name is required", validate: validatePersonName, - maxLength: { - value: 50, - message: "First name must be within 50 characters.", - }, }}Apply the same pattern to
last_nameat lines 336-343.apps/web/core/components/profile/form.tsx (1)
26-27: Consolidate imports from@plane/utils.There are two separate imports from
@plane/utils(line 14 and line 27). Consider merging them:♻️ Suggested consolidation
-import { getFileURL } from "@plane/utils"; +import { getFileURL, validatePersonName, validateDisplayName } from "@plane/utils"; // components ... -// utils -import { validatePersonName, validateDisplayName } from "@plane/utils";apps/admin/core/components/instance/setup-form.tsx (1)
170-178: Silent input blocking may confuse users.The validation blocks invalid characters by simply not updating state, which means users won't see any feedback when typing rejected characters. Consider showing a brief error message or toast to inform users why their input isn't appearing.
Additionally, the
last_namefield is marked as required in the UI (Line 183:<span className="text-danger-primary">*</span>) but isn't validated as required on form submission - onlyfirst_nameis checked inisButtonDisabled.packages/utils/src/validation.ts (1)
14-20: Support Unicode letters in person names for international names.
PERSON_NAME_REGEXcurrently only allows ASCII letters (a-zA-Z), which excludes valid international names with accented characters (e.g., "José", "François", "Müller"). Since the project targets es2023, you can use Unicode property escapes:-export const PERSON_NAME_REGEX = /^[a-zA-Z\s'-]+$/; +export const PERSON_NAME_REGEX = /^[\p{L}\s'-]+$/u;The
\p{L}matches any Unicode letter with theuflag for Unicode mode.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/admin/core/components/instance/setup-form.tsxapps/web/core/components/onboarding/create-workspace.tsxapps/web/core/components/onboarding/profile-setup.tsxapps/web/core/components/onboarding/steps/profile/root.tsxapps/web/core/components/onboarding/steps/workspace/create.tsxapps/web/core/components/profile/form.tsxapps/web/core/components/workspace/create-workspace-form.tsxpackages/utils/src/index.tspackages/utils/src/validation.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,mts,cts}
📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)
**/*.{ts,tsx,mts,cts}: Useconsttype parameters for more precise literal inference in TypeScript 5.0+
Use thesatisfiesoperator to validate types without widening them
Leverage inferred type predicates to reduce the need for explicitisreturn types in filter/check functions
UseNoInfer<T>utility to block inference for specific type arguments when they should be determined by other arguments
Utilize narrowing inswitch(true)blocks for control flow analysis (TypeScript 5.3+)
Rely on narrowing from direct boolean comparisons for type guards
Trust preserved narrowing in closures when variables aren't modified after the check (TypeScript 5.4+)
Use constant indices to narrow object/array properties (TypeScript 5.5+)
Use standard ECMAScript decorators (Stage 3) instead of legacyexperimentalDecorators
Useusingdeclarations for explicit resource management with Disposable pattern instead of manual cleanup (TypeScript 5.2+)
Usewith { type: "json" }for import attributes; avoid deprecatedassertsyntax (TypeScript 5.3/5.8+)
Useimport typeexplicitly when importing types to ensure they are erased during compilation, respectingverbatimModuleSyntaxflag
Use.ts,.mts,.ctsextensions inimport typestatements (TypeScript 5.2+)
Useimport type { Type } from "mod" with { "resolution-mode": "import" }for specific module resolution contexts (TypeScript 5.3+)
Use new iterator methods (map, filter, etc.) if targeting modern environments (TypeScript 5.6+)
Utilize newSetmethods likeunion,intersection, etc., when available (TypeScript 5.5+)
UseObject.groupBy/Map.groupBystandard methods for grouping instead of external libraries (TypeScript 5.4+)
UsePromise.withResolvers()for creating promises with exposed resolve/reject functions (TypeScript 5.7+)
Use copying array methods (toSorted,toSpliced,with) for immutable array operations (TypeScript 5.2+)
Avoid accessing instance fields viasuperin classes (TypeScript 5....
Files:
apps/web/core/components/onboarding/profile-setup.tsxapps/web/core/components/onboarding/create-workspace.tsxapps/web/core/components/profile/form.tsxpackages/utils/src/validation.tsapps/web/core/components/onboarding/steps/workspace/create.tsxapps/web/core/components/onboarding/steps/profile/root.tsxpackages/utils/src/index.tsapps/web/core/components/workspace/create-workspace-form.tsxapps/admin/core/components/instance/setup-form.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Enable TypeScript strict mode and ensure all files are fully typed
Files:
apps/web/core/components/onboarding/profile-setup.tsxapps/web/core/components/onboarding/create-workspace.tsxapps/web/core/components/profile/form.tsxpackages/utils/src/validation.tsapps/web/core/components/onboarding/steps/workspace/create.tsxapps/web/core/components/onboarding/steps/profile/root.tsxpackages/utils/src/index.tsapps/web/core/components/workspace/create-workspace-form.tsxapps/admin/core/components/instance/setup-form.tsx
**/*.{js,jsx,ts,tsx,json,css}
📄 CodeRabbit inference engine (AGENTS.md)
Use Prettier with Tailwind plugin for code formatting, run
pnpm fix:format
Files:
apps/web/core/components/onboarding/profile-setup.tsxapps/web/core/components/onboarding/create-workspace.tsxapps/web/core/components/profile/form.tsxpackages/utils/src/validation.tsapps/web/core/components/onboarding/steps/workspace/create.tsxapps/web/core/components/onboarding/steps/profile/root.tsxpackages/utils/src/index.tsapps/web/core/components/workspace/create-workspace-form.tsxapps/admin/core/components/instance/setup-form.tsx
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,jsx,ts,tsx}: Use ESLint with shared config across packages, adhering to max warnings limits per package
Use camelCase for variable and function names, PascalCase for components and types
Use try-catch with proper error types and log errors appropriately
Files:
apps/web/core/components/onboarding/profile-setup.tsxapps/web/core/components/onboarding/create-workspace.tsxapps/web/core/components/profile/form.tsxpackages/utils/src/validation.tsapps/web/core/components/onboarding/steps/workspace/create.tsxapps/web/core/components/onboarding/steps/profile/root.tsxpackages/utils/src/index.tsapps/web/core/components/workspace/create-workspace-form.tsxapps/admin/core/components/instance/setup-form.tsx
🧠 Learnings (6)
📓 Common learnings
Learnt from: NarayanBavisetti
Repo: makeplane/plane PR: 7460
File: apps/api/plane/app/serializers/draft.py:112-122
Timestamp: 2025-07-23T18:18:06.875Z
Learning: In the Plane codebase serializers, workspace_id is not consistently passed in serializer context, so parent issue validation in DraftIssueCreateSerializer only checks project_id rather than both workspace_id and project_id. The existing project member authentication system already validates that users can only access projects they belong to, providing sufficient security without risking breaking functionality by adding workspace_id validation where the context might not be available.
📚 Learning: 2025-10-01T15:30:17.605Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7888
File: packages/propel/src/avatar/avatar.stories.tsx:2-3
Timestamp: 2025-10-01T15:30:17.605Z
Learning: In the makeplane/plane repository, avoid suggesting inline type imports (e.g., `import { Avatar, type TAvatarSize }`) due to bundler compatibility issues. Keep type imports and value imports as separate statements.
Applied to files:
apps/web/core/components/onboarding/profile-setup.tsxapps/web/core/components/onboarding/create-workspace.tsx
📚 Learning: 2025-10-21T17:22:05.204Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7989
File: apps/web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(detail)/[pageId]/page.tsx:45-46
Timestamp: 2025-10-21T17:22:05.204Z
Learning: In the makeplane/plane repository, the refactor from useParams() to params prop is specifically scoped to page.tsx and layout.tsx files in apps/web/app (Next.js App Router pattern). Other components (hooks, regular client components, utilities) should continue using the useParams() hook as that is the correct pattern for non-route components.
Applied to files:
apps/web/core/components/onboarding/profile-setup.tsxapps/web/core/components/onboarding/create-workspace.tsxapps/web/core/components/workspace/create-workspace-form.tsxapps/admin/core/components/instance/setup-form.tsx
📚 Learning: 2025-07-23T18:18:06.875Z
Learnt from: NarayanBavisetti
Repo: makeplane/plane PR: 7460
File: apps/api/plane/app/serializers/draft.py:112-122
Timestamp: 2025-07-23T18:18:06.875Z
Learning: In the Plane codebase serializers, workspace_id is not consistently passed in serializer context, so parent issue validation in DraftIssueCreateSerializer only checks project_id rather than both workspace_id and project_id. The existing project member authentication system already validates that users can only access projects they belong to, providing sufficient security without risking breaking functionality by adding workspace_id validation where the context might not be available.
Applied to files:
apps/web/core/components/onboarding/create-workspace.tsx
📚 Learning: 2025-09-02T08:14:49.260Z
Learnt from: sriramveeraghanta
Repo: makeplane/plane PR: 7697
File: apps/web/app/(all)/[workspaceSlug]/(projects)/header.tsx:12-13
Timestamp: 2025-09-02T08:14:49.260Z
Learning: The star-us-link.tsx file in apps/web/app/(all)/[workspaceSlug]/(projects)/ already has "use client" directive at the top, making it a proper Client Component for hook usage.
Applied to files:
apps/web/core/components/onboarding/create-workspace.tsxapps/web/core/components/onboarding/steps/workspace/create.tsxapps/web/core/components/workspace/create-workspace-form.tsx
📚 Learning: 2025-12-12T15:20:36.542Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-12T15:20:36.542Z
Learning: Applies to **/package.json : Use `workspace:*` for internal packages and `catalog:` for external dependencies in imports
Applied to files:
apps/web/core/components/onboarding/steps/workspace/create.tsxpackages/utils/src/index.ts
🧬 Code graph analysis (7)
apps/web/core/components/onboarding/profile-setup.tsx (1)
packages/utils/src/validation.ts (1)
validatePersonName(60-74)
apps/web/core/components/onboarding/create-workspace.tsx (1)
packages/utils/src/validation.ts (2)
validateWorkspaceName(138-152)validateSlug(163-177)
apps/web/core/components/profile/form.tsx (1)
packages/utils/src/validation.ts (2)
validatePersonName(60-74)validateDisplayName(86-100)
apps/web/core/components/onboarding/steps/workspace/create.tsx (1)
packages/utils/src/validation.ts (2)
validateWorkspaceName(138-152)validateSlug(163-177)
apps/web/core/components/onboarding/steps/profile/root.tsx (1)
packages/utils/src/validation.ts (1)
validatePersonName(60-74)
apps/web/core/components/workspace/create-workspace-form.tsx (1)
packages/utils/src/validation.ts (2)
validateWorkspaceName(138-152)validateSlug(163-177)
apps/admin/core/components/instance/setup-form.tsx (1)
packages/utils/src/validation.ts (2)
validatePersonName(60-74)validateCompanyName(112-126)
⏰ 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). (2)
- GitHub Check: Build packages
- GitHub Check: Analyze (javascript)
🔇 Additional comments (3)
packages/utils/src/index.ts (1)
33-33: LGTM!The barrel export correctly exposes the new validation module, following the existing alphabetical ordering convention.
packages/utils/src/validation.ts (2)
138-152: LGTM - Separate validator for workspace names.The intentional separation from
validateCompanyNameallows workspace-specific error messages while reusing the same regex pattern. This is a reasonable trade-off for UX clarity.
1-48: Well-structured validation module with good documentation.The OWASP-aligned allowlist approach, clear regex pattern documentation, and JSDoc examples make this a solid foundation for centralized input validation. The
boolean | stringreturn type integrates well with react-hook-form's validation system.
| onChange={(e) => { | ||
| const validation = validateCompanyName(e.target.value, false); | ||
| if (validation === true || e.target.value === "") { | ||
| handleFormChange("company_name", e.target.value); | ||
| } | ||
| }} | ||
| maxLength={80} |
There was a problem hiding this comment.
Company name required parameter inconsistent with UI.
The UI marks company name as required (Line 228 shows the asterisk), but validateCompanyName is called with required: false. If the field is truly required, pass true to enforce validation:
- const validation = validateCompanyName(e.target.value, false);
+ const validation = validateCompanyName(e.target.value, true);🤖 Prompt for AI Agents
In @apps/admin/core/components/instance/setup-form.tsx around lines 238 - 244,
The company name field is marked required in the UI but validateCompanyName is
being called with required: false; update the onChange handler so
validateCompanyName is called with required: true (instead of false) so the
required validation is enforced before calling handleFormChange for
"company_name"; ensure the rest of the handler behavior remains the same and
that maxLength/empty-string allowance logic still matches your intended UX.
| onChange={(e) => { | ||
| if (/^[a-zA-Z0-9_-]+$/.test(e.target.value)) setInvalidSlug(false); | ||
| const validation = validateSlug(e.target.value); | ||
| if (validation === true) setInvalidSlug(false); | ||
| else setInvalidSlug(true); | ||
| onChange(e.target.value.toLowerCase()); |
There was a problem hiding this comment.
Same slug validation issue as in steps/workspace/create.tsx.
The slug validation runs on the raw input (e.target.value) while the displayed value is transformed (Line 195). Validate the transformed value for consistency:
onChange={(e) => {
- const validation = validateSlug(e.target.value);
+ const transformedSlug = e.target.value.toLowerCase().trim().replace(/ /g, "-");
+ const validation = validateSlug(transformedSlug);
if (validation === true) setInvalidSlug(false);
else setInvalidSlug(true);
onChange(e.target.value.toLowerCase());
}}🤖 Prompt for AI Agents
In @apps/web/core/components/onboarding/create-workspace.tsx around lines 196 -
200, The slug validation is using the raw input value instead of the
transformed/displayed slug; update the onChange handler in the create-workspace
component so it computes the transformedSlug (e.g., const transformed =
e.target.value.toLowerCase()) and run validateSlug(transformed) before calling
setInvalidSlug and onChange; specifically adjust the handler around
validateSlug, setInvalidSlug, and onChange to operate on the transformed value
rather than e.target.value.
| onChange={(e) => { | ||
| if (/^[a-zA-Z0-9_-]+$/.test(e.target.value)) setInvalidSlug(false); | ||
| const validation = validateSlug(e.target.value); | ||
| if (validation === true) setInvalidSlug(false); | ||
| else setInvalidSlug(true); | ||
| onChange(e.target.value.toLowerCase()); |
There was a problem hiding this comment.
Slug validation runs on raw input before transformation.
The displayed slug value applies transformations (lowercase, trim, replace spaces with hyphens on Line 213), but validateSlug on Line 215 validates e.target.value directly. This means typing a space will fail validation even though it would be transformed to a hyphen in the display.
Consider validating the transformed value instead:
onChange={(e) => {
- const validation = validateSlug(e.target.value);
+ const transformedSlug = e.target.value.toLowerCase().trim().replace(/ /g, "-");
+ const validation = validateSlug(transformedSlug);
if (validation === true) setInvalidSlug(false);
else setInvalidSlug(true);
onChange(e.target.value.toLowerCase());
}}📝 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.
| onChange={(e) => { | |
| if (/^[a-zA-Z0-9_-]+$/.test(e.target.value)) setInvalidSlug(false); | |
| const validation = validateSlug(e.target.value); | |
| if (validation === true) setInvalidSlug(false); | |
| else setInvalidSlug(true); | |
| onChange(e.target.value.toLowerCase()); | |
| onChange={(e) => { | |
| const transformedSlug = e.target.value.toLowerCase().trim().replace(/ /g, "-"); | |
| const validation = validateSlug(transformedSlug); | |
| if (validation === true) setInvalidSlug(false); | |
| else setInvalidSlug(true); | |
| onChange(e.target.value.toLowerCase()); | |
| }} |
🤖 Prompt for AI Agents
In @apps/web/core/components/onboarding/steps/workspace/create.tsx around lines
214 - 218, The handler is validating the raw input instead of the
displayed/transformed slug; compute the transformed slug first (same logic used
for display: trim, toLowerCase, replace spaces with hyphens), then call
validateSlug on that transformed value, setInvalidSlug based on that result, and
pass the transformed slug into onChange instead of e.target.value so validation
matches the shown value (refer to validateSlug, setInvalidSlug, and the onChange
handler).
| rules={{ | ||
| validate: validatePersonName, | ||
| }} |
There was a problem hiding this comment.
Validation mismatch: last_name appears optional but validation requires it.
The last_name field label (line 281) has no required asterisk, suggesting it's optional. However, validatePersonName returns "Name is required" for empty values, effectively making last_name mandatory and blocking form submission when empty.
Either mark the field as required in the UI, or use a validator that permits empty values:
🔧 Option 1: Create an optional person name validator
In validation.ts, add an optional variant:
export const validateOptionalPersonName = (name: string): boolean | string => {
if (!name || name.trim() === "") {
return true; // Allow empty
}
// Rest of validation...
if (name.length > 50) {
return "Name must be 50 characters or less";
}
if (!PERSON_NAME_REGEX.test(name)) {
return "Names can only contain letters, spaces, hyphens, and apostrophes";
}
return true;
};Then use it here:
rules={{
- validate: validatePersonName,
+ validate: validateOptionalPersonName,
}}🔧 Option 2: Mark last_name as required in UI
-<h4 className="text-13 font-medium text-secondary">{t("last_name")}</h4>
+<h4 className="text-13 font-medium text-secondary">
+ {t("last_name")}
+ <span className="text-danger-primary">*</span>
+</h4>🤖 Prompt for AI Agents
In @apps/web/core/components/profile/form.tsx around lines 285 - 287, The
last_name field label is presented as optional but its rules use
validatePersonName which rejects empty values; either make the field truly
optional by adding a validator that permits empty input (e.g., create
validateOptionalPersonName in validation.ts that returns true for
empty/whitespace and otherwise enforces the same length/regex checks as
validatePersonName, then swap validatePersonName for validateOptionalPersonName
in the rules) or make the field required in the UI (update the label to indicate
required and add a required rule alongside validatePersonName). Ensure you
reference and update validatePersonName/validateOptionalPersonName and the
last_name field's rules/label accordingly.
| export const hasInjectionRiskChars = (input: string): boolean => { | ||
| const injectionRiskPattern = /[<>'"{}[\]*^!#%]/; | ||
| return injectionRiskPattern.test(input); | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for usages of hasInjectionRiskChars across the entire codebase
rg -n "hasInjectionRiskChars" --type=ts --type=tsxRepository: makeplane/plane
Length of output: 174
🏁 Script executed:
#!/bin/bash
# Also check for any imports of this function
rg -n "hasInjectionRiskChars" --type=ts --type=tsx --type=js --type=jsxRepository: makeplane/plane
Length of output: 86
🏁 Script executed:
#!/bin/bash
# Search for hasInjectionRiskChars in the codebase without type restrictions
rg "hasInjectionRiskChars" --max-count=20Repository: makeplane/plane
Length of output: 339
🏁 Script executed:
#!/bin/bash
# Also check if it's re-exported from index files
rg "hasInjectionRiskChars" -A 2 -B 2Repository: makeplane/plane
Length of output: 676
🏁 Script executed:
#!/bin/bash
# Check the validation.ts file to see the complete context
cat packages/utils/src/validation.ts | grep -A 10 -B 5 "hasInjectionRiskChars"Repository: makeplane/plane
Length of output: 536
🏁 Script executed:
#!/bin/bash
# Check if hasInjectionRiskChars is exported from any index/barrel files
cat packages/utils/src/index.ts | grep -i "validation\|hasInjectionRiskChars"Repository: makeplane/plane
Length of output: 88
🏁 Script executed:
#!/bin/bash
# List the contents of the utils src directory to understand the module structure
ls -la packages/utils/src/Repository: makeplane/plane
Length of output: 2513
🏁 Script executed:
#!/bin/bash
# Search for any imports of hasInjectionRiskChars or usage of the function outside validation.ts
rg "hasInjectionRiskChars" --no-headingRepository: makeplane/plane
Length of output: 339
🏁 Script executed:
#!/bin/bash
# Check if it's used in any test files
find . -name "*.test.*" -o -name "*.spec.*" | head -20Repository: makeplane/plane
Length of output: 165
🏁 Script executed:
#!/bin/bash
# Search more broadly for any reference to hasInjectionRiskChars excluding the validation.ts definition
rg "hasInjectionRiskChars" -lRepository: makeplane/plane
Length of output: 91
Remove hasInjectionRiskChars as it is dead code.
This utility function is exported but has zero usages in the entire codebase. Unless it's intentionally part of a public API for external consumers, it should be removed to reduce maintenance burden.
🤖 Prompt for AI Agents
In @packages/utils/src/validation.ts around lines 187 - 190, Remove the dead
exported function hasInjectionRiskChars (and its internal injectionRiskPattern)
from packages/utils/src/validation.ts; delete its export and any associated
declarations, and update any barrel exports or public API lists (e.g., index
files) that re-export hasInjectionRiskChars so the codebase no longer references
an unused symbol.
|
Linked to Plane Work Item(s) This comment was auto-generated by Plane |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/admin/app/(all)/(dashboard)/workspace/create/form.tsx (1)
126-150: Remove redundant inline slug validation.The
validateSlugutility already validates the slug format, making the inline regex check (lines 134-135) and theinvalidSlugstate redundant. This creates potential for duplicate error messages—for an empty slug, botherrors.slug.message("Slug is required") and theinvalidSlugerror could display.♻️ Suggested refactor to remove redundant validation
render={({ field: { onChange, value, ref } }) => ( <Input id="workspaceUrl" type="text" value={value.toLocaleLowerCase().trim().replace(/ /g, "-")} onChange={(e) => { - if (/^[a-zA-Z0-9_-]+$/.test(e.target.value)) setInvalidSlug(false); - else setInvalidSlug(true); onChange(e.target.value.toLowerCase()); }} ref={ref} hasError={Boolean(errors.slug)} placeholder="workspace-name" className="block w-full rounded-md border-none bg-transparent !px-0 py-2 text-13" /> )} /> </div> {slugError && <p className="text-13 text-danger-primary">This URL is taken. Try something else.</p>} - {invalidSlug && ( - <p className="text-13 text-danger-primary">{`URLs can contain only ( - ), ( _ ) and alphanumeric characters.`}</p> - )} {errors.slug && <span className="text-11 text-danger-primary">{errors.slug.message}</span>}Also remove the
invalidSlugstate at line 24:const [slugError, setSlugError] = useState(false); - const [invalidSlug, setInvalidSlug] = useState(false);
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/admin/app/(all)/(dashboard)/workspace/create/form.tsxapps/web/core/components/workspace/settings/workspace-details.tsx
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,mts,cts}
📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)
**/*.{ts,tsx,mts,cts}: Useconsttype parameters for more precise literal inference in TypeScript 5.0+
Use thesatisfiesoperator to validate types without widening them
Leverage inferred type predicates to reduce the need for explicitisreturn types in filter/check functions
UseNoInfer<T>utility to block inference for specific type arguments when they should be determined by other arguments
Utilize narrowing inswitch(true)blocks for control flow analysis (TypeScript 5.3+)
Rely on narrowing from direct boolean comparisons for type guards
Trust preserved narrowing in closures when variables aren't modified after the check (TypeScript 5.4+)
Use constant indices to narrow object/array properties (TypeScript 5.5+)
Use standard ECMAScript decorators (Stage 3) instead of legacyexperimentalDecorators
Useusingdeclarations for explicit resource management with Disposable pattern instead of manual cleanup (TypeScript 5.2+)
Usewith { type: "json" }for import attributes; avoid deprecatedassertsyntax (TypeScript 5.3/5.8+)
Useimport typeexplicitly when importing types to ensure they are erased during compilation, respectingverbatimModuleSyntaxflag
Use.ts,.mts,.ctsextensions inimport typestatements (TypeScript 5.2+)
Useimport type { Type } from "mod" with { "resolution-mode": "import" }for specific module resolution contexts (TypeScript 5.3+)
Use new iterator methods (map, filter, etc.) if targeting modern environments (TypeScript 5.6+)
Utilize newSetmethods likeunion,intersection, etc., when available (TypeScript 5.5+)
UseObject.groupBy/Map.groupBystandard methods for grouping instead of external libraries (TypeScript 5.4+)
UsePromise.withResolvers()for creating promises with exposed resolve/reject functions (TypeScript 5.7+)
Use copying array methods (toSorted,toSpliced,with) for immutable array operations (TypeScript 5.2+)
Avoid accessing instance fields viasuperin classes (TypeScript 5....
Files:
apps/admin/app/(all)/(dashboard)/workspace/create/form.tsxapps/web/core/components/workspace/settings/workspace-details.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Enable TypeScript strict mode and ensure all files are fully typed
Files:
apps/admin/app/(all)/(dashboard)/workspace/create/form.tsxapps/web/core/components/workspace/settings/workspace-details.tsx
**/*.{js,jsx,ts,tsx,json,css}
📄 CodeRabbit inference engine (AGENTS.md)
Use Prettier with Tailwind plugin for code formatting, run
pnpm fix:format
Files:
apps/admin/app/(all)/(dashboard)/workspace/create/form.tsxapps/web/core/components/workspace/settings/workspace-details.tsx
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,jsx,ts,tsx}: Use ESLint with shared config across packages, adhering to max warnings limits per package
Use camelCase for variable and function names, PascalCase for components and types
Use try-catch with proper error types and log errors appropriately
Files:
apps/admin/app/(all)/(dashboard)/workspace/create/form.tsxapps/web/core/components/workspace/settings/workspace-details.tsx
🧠 Learnings (1)
📓 Common learnings
Learnt from: NarayanBavisetti
Repo: makeplane/plane PR: 7460
File: apps/api/plane/app/serializers/draft.py:112-122
Timestamp: 2025-07-23T18:18:06.875Z
Learning: In the Plane codebase serializers, workspace_id is not consistently passed in serializer context, so parent issue validation in DraftIssueCreateSerializer only checks project_id rather than both workspace_id and project_id. The existing project member authentication system already validates that users can only access projects they belong to, providing sufficient security without risking breaking functionality by adding workspace_id validation where the context might not be available.
🧬 Code graph analysis (2)
apps/admin/app/(all)/(dashboard)/workspace/create/form.tsx (1)
packages/utils/src/validation.ts (2)
validateWorkspaceName(138-152)validateSlug(163-177)
apps/web/core/components/workspace/settings/workspace-details.tsx (1)
packages/utils/src/validation.ts (1)
validateWorkspaceName(138-152)
⏰ 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). (3)
- GitHub Check: check:types
- GitHub Check: check:lint
- GitHub Check: Analyze (javascript)
🔇 Additional comments (4)
apps/admin/app/(all)/(dashboard)/workspace/create/form.tsx (2)
11-11: LGTM!Good use of centralized validation utilities from
@plane/utils, aligning with the PR objective of consistent OWASP-compliant input validation across the codebase.
94-94: LGTM!The validation is correctly delegated to the centralized
validateWorkspaceNameutility withrequired: true.apps/web/core/components/workspace/settings/workspace-details.tsx (2)
12-12: LGTM!Appropriate import of the centralized validation utility.
187-208: LGTM!Good implementation of the centralized validation with proper error display. The validation correctly enforces workspace name rules (required, max 80 chars, allowed characters) via the shared utility.
Description
Type of Change
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.