fix(ui): validate webhook fields in install-key drawers - #6866
Conversation
|
/review |
|
Claude finished @otavio's task in 4m 3s —— View job Code Review CompleteReviewed 5 files across code quality, security, testing, language patterns, and architecture. 1 inline issue posted (1 medium). The URL/secret/numeric validation changes themselves are solid —
If you've addressed the feedback and want a new review, tag |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
The webhook URL field only checked for an `http://` or `https://` prefix via regex, so structurally invalid URLs like `https://` (no host) passed. The signing secret used a plain `InputField type="password"` with no visibility toggle. The reply timeout and callback window used `InputField type="number"`, which accepts non-numeric characters (`e`, `+`, `-`, `.`) and rendered browser-native spinners. - Replace the URL regex with `new URL()` + protocol check (`isWebhookUrl` in `helpers.ts`); add inline blur-validation error - Swap the secret to `PasswordField` with `suppressPasswordManager`; show a "leave blank to keep" hint when editing an existing webhook key - Swap timeout and callback window to `NumericInput` with digit-only filtering, inline range errors, and eager parent sync so `validateModeConfig` always sees the current typed value and blocks submit while out of range - Extract range constants (`TIMEOUT_MIN/MAX`, `WINDOW_MIN_H/MAX_H`) shared between inline errors and `validateModeConfig` - Fix mobile layout: replace `grid-cols-2` with `flex flex-wrap` so the two numeric fields stack when the drawer is narrow
a5a1a3c to
a33ac94
Compare
|
/review |
|
Claude finished @otavio's task in 4m 10s —— View job Code Review CompleteReviewed 5 files across code quality, security, testing, language patterns, and architecture — no new issues found. The code looks good as-is. The previously-flagged state-lifecycle concern (stale numeric/ If you push additional changes and want a new review, tag |
2 participants
What
Replaced the four webhook-mode fields in the install-key drawers (Create and Edit) with purpose-built components that properly validate input and block submit on invalid values.
Why
The webhook URL only checked for an
http:///https://prefix via regex — structurally invalid URLs likehttps://(no host) passed client-side validation. The signing secret was a plainInputField type="password"with no visibility toggle. The reply timeout and callback window usedInputField type="number", which accepts non-numeric characters (e,+,-,.) past the browser's native spinner and rendered inconsistently across browsers. On mobile, the two-columngrid-cols-2layout overflowed the drawer.Changes
helpers.ts,ModeField.tsx): replaced the prefix regex invalidateModeConfigwithisWebhookUrl, which usesnew URL()+ protocol check. The URL field now shows an inline error on blur when the value is non-empty and structurally invalid.ModeField.tsx): swappedInputField type="password"forPasswordFieldwithsuppressPasswordManager. In edit mode (isEditingprop), the hint tells the user to leave the field blank to keep the current secret.ModeField.tsx): swappedInputField type="number"forNumericInput(digit-only regex filter viatype="text"+inputMode="numeric"). Each field shows an inline range error and eagerly syncs the parsed value to the parent on every keystroke, sovalidateModeConfigalways sees the current typed value and disables the submit button while out of range. On blur, the value clamps to the valid range.helpers.ts): extractedTIMEOUT_MIN/MAXandWINDOW_MIN_H/MAX_Hso inline errors andvalidateModeConfigshare a single source of truth.ModeField.tsx): replacedgrid grid-cols-2withflex flex-wrap gap-3so the timeout and callback fields stack when the drawer is narrow.Testing
isWebhookUrl: 8 cases (valid URLs, protocol-only, wrong protocol, empty, whitespace)validateModeConfig: updated to rejecthttps://(no host); new cases for out-of-range timeout (0, 16) and callback (0, 25h)