Neutralize formula characters in csv exports#4128
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughA shared ChangesPartner Field Sanitization
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai full review |
✅ Action performedFull review finished. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/lib/actions/partners/update-partner-profile.ts`:
- Around line 33-38: The partner profile schema’s name validation has the same
required-then-sanitized ordering problem as the onboarding schema, so a
formula-only input can become empty after sanitization and overwrite the
existing value. Update the `update-partner-profile.ts` schema `name` field (and
align it with `onboardPartnerSchema.name` if shared) so `sanitizeFormulaInput`
runs before enforcing non-empty content, or add a post-transform
`refine`/equivalent check after `sanitizeFormulaInput` to reject empty results
while preserving the optional behavior.
In `@apps/web/lib/zod/schemas/partners.ts`:
- Around line 752-760: The sanitizeFormulaInput helper currently removes formula
prefixes before leading whitespace, so values like “ =...” can still end up
looking like formulas. Update sanitizeFormulaInput to strip leading whitespace
first or include it in the same leading removal, then ensure the name,
description, and companyName transforms in partners.ts use this helper so all
three fields get the corrected sanitization.
- Line 770: The name field validation in the partners schema allows values that
become empty after sanitizeFormulaInput because z.string().min(1) runs before
the transform. Update the name pipeline in the partners schema to revalidate
after sanitizeFormulaInput, matching the companyName pattern, so the sanitized
result must still be non-empty.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ac186c41-29c9-4db4-83dc-8a2bcc8dbd75
📒 Files selected for processing (2)
apps/web/lib/actions/partners/update-partner-profile.tsapps/web/lib/zod/schemas/partners.ts
There was a problem hiding this comment.
♻️ Duplicate comments (3)
apps/web/lib/zod/schemas/partners.ts (2)
752-760: 🔒 Security & Privacy | 🔴 Critical | ⚡ Quick winFormula-char strip still bypassable via leading whitespace.
replace(/^[=+\-@\t\r]+/, "")only matches at index 0 and the class excludes plain space/newline, so a leading space blocks the match;.trimStart()then removes that space and re-exposes the formula trigger (" =cmd|calc!A0"→"=cmd|calc!A0"). This is the exact bug flagged in a prior review on this file and marked "Addressed", but the code here is unchanged from the vulnerable version —descriptionandcompanyName(which don't get a.trim()upstream) remain exploitable, defeating the purpose of this PR.🐛 Proposed fix
- return value.replace(/^[=+\-@\t\r]+/, "").trimStart() as T; + return value.replace(/^[\s=+\-@]+/, "") as T;Merging whitespace into the same leading character class strips interleaved whitespace/formula-char runs (e.g.
" = A1") in one pass instead of two ordered passes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/lib/zod/schemas/partners.ts` around lines 752 - 760, The sanitizeFormulaInput helper still allows a leading space/newline to hide a formula trigger because the regex only strips characters at index 0 and trimStart runs afterward. Update sanitizeFormulaInput in partners.ts so the leading-stripping step handles whitespace and formula-leading characters together in one pass, and keep the behavior applied to fields like description and companyName that do not get trimmed upstream.
770-770: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winRevalidate
nameafter sanitizing.
z.string().min(1)runs before.transform(sanitizeFormulaInput), so a submission like"="passes the required check and is then sanitized to"", silently bypassing the "Name is required" constraint. This matches a still-open concern from a prior review pass on this exact line.🐛 Proposed fix
- name: z.string().min(1, "Name is required").transform(sanitizeFormulaInput), + name: z + .string() + .min(1, "Name is required") + .transform(sanitizeFormulaInput) + .refine((v) => v.length > 0, { message: "Name is required" }),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/lib/zod/schemas/partners.ts` at line 770, The `name` field in the `partners` Zod schema is only validated before `sanitizeFormulaInput` runs, so a value like "=" can become empty after transformation and bypass the required check. Update the `name` pipeline in the partners schema to validate again after `sanitizeFormulaInput` (for example by moving the required validation to the sanitized result or adding a post-transform refinement) so the final value cannot be empty. Use the `name` schema entry in `partners.ts` as the location to adjust.apps/web/lib/actions/partners/update-partner-profile.ts (1)
33-38: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winSame required-then-emptied ordering issue as onboarding schema's
namefield.
.min(1, "Name is required")runs before.transform(sanitizeFormulaInput). A submittednameof purely formula-trigger characters (e.g."=") passes the required check, then sanitizes to"", silently overwriting the partner's existing name. Same class of issue asapps/web/lib/zod/schemas/partners.ts'sonboardPartnerSchema.name, previously flagged here and still unresolved.🐛 Proposed fix
name: z .string() .trim() .min(1, "Name is required") .optional() - .transform(sanitizeFormulaInput), + .transform(sanitizeFormulaInput) + .refine((v) => v === undefined || v.length > 0, { + message: "Name is required", + }),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/lib/actions/partners/update-partner-profile.ts` around lines 33 - 38, The partner profile `name` schema in `update-partner-profile.ts` has the same required-then-sanitized ordering bug as `onboardPartnerSchema.name`: the `z.string().trim().min(1, "Name is required").optional().transform(sanitizeFormulaInput)` chain validates before sanitizing, so formula-only input can become empty after transform and overwrite the existing name. Update this field so `sanitizeFormulaInput` runs before the non-empty validation, matching the fix pattern used for the onboarding schema, and keep the `name` field in sync with the corresponding partner schema definitions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@apps/web/lib/actions/partners/update-partner-profile.ts`:
- Around line 33-38: The partner profile `name` schema in
`update-partner-profile.ts` has the same required-then-sanitized ordering bug as
`onboardPartnerSchema.name`: the `z.string().trim().min(1, "Name is
required").optional().transform(sanitizeFormulaInput)` chain validates before
sanitizing, so formula-only input can become empty after transform and overwrite
the existing name. Update this field so `sanitizeFormulaInput` runs before the
non-empty validation, matching the fix pattern used for the onboarding schema,
and keep the `name` field in sync with the corresponding partner schema
definitions.
In `@apps/web/lib/zod/schemas/partners.ts`:
- Around line 752-760: The sanitizeFormulaInput helper still allows a leading
space/newline to hide a formula trigger because the regex only strips characters
at index 0 and trimStart runs afterward. Update sanitizeFormulaInput in
partners.ts so the leading-stripping step handles whitespace and formula-leading
characters together in one pass, and keep the behavior applied to fields like
description and companyName that do not get trimmed upstream.
- Line 770: The `name` field in the `partners` Zod schema is only validated
before `sanitizeFormulaInput` runs, so a value like "=" can become empty after
transformation and bypass the required check. Update the `name` pipeline in the
partners schema to validate again after `sanitizeFormulaInput` (for example by
moving the required validation to the sanitized result or adding a
post-transform refinement) so the final value cannot be empty. Use the `name`
schema entry in `partners.ts` as the location to adjust.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cdf4a93b-2563-4173-82e4-c1b6ddc3e97d
📒 Files selected for processing (2)
apps/web/lib/actions/partners/update-partner-profile.tsapps/web/lib/zod/schemas/partners.ts
2 participants
Summary by CodeRabbit