Skip to content

Commit 601ed9b

Browse files
authored
fix: network isolation form validation (#637)
* fix: enforce type number for port field * fix: show specific validation message * fix: validate form on change * test: update
1 parent cd6594a commit 601ed9b

File tree

5 files changed

+29
-23
lines changed

5 files changed

+29
-23
lines changed

renderer/src/features/registry-servers/components/__tests__/form-run-from-registry.test.tsx

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -970,6 +970,13 @@ describe('FormRunFromRegistry', () => {
970970
await userEvent.click(addPortButton)
971971
await userEvent.type(screen.getByLabelText('Port 1'), '8080')
972972
await userEvent.click(addPortButton)
973+
await userEvent.type(screen.getByLabelText('Port 2'), '666666666')
974+
await waitFor(() => {
975+
expect(
976+
screen.getByText(/port must be a number between 1 and 65535/i)
977+
).toBeInTheDocument()
978+
})
979+
await userEvent.clear(screen.getByLabelText('Port 2'))
973980
await userEvent.type(screen.getByLabelText('Port 2'), '443')
974981
// Submit
975982
const configTab = screen.getByRole('tab', { name: /configuration/i })
@@ -1151,7 +1158,7 @@ describe('Allowed Hosts field', () => {
11511158
await userEvent.click(
11521159
screen.getByRole('button', { name: /install server/i })
11531160
)
1154-
expect(screen.getByText(/invalid host/i)).toBeInTheDocument()
1161+
expect(screen.getByText(/invalid host format/i)).toBeInTheDocument()
11551162
// Valid host
11561163
let hostInputRef = screen.queryByLabelText('Host 1')
11571164
if (hostInputRef) {
@@ -1161,7 +1168,7 @@ describe('Allowed Hosts field', () => {
11611168
await userEvent.click(
11621169
screen.getByRole('button', { name: /install server/i })
11631170
)
1164-
expect(screen.queryByText(/invalid host/i)).not.toBeInTheDocument()
1171+
expect(screen.queryByText(/invalid host format/i)).not.toBeInTheDocument()
11651172
}
11661173
// Valid host with dot
11671174
hostInputRef = screen.queryByLabelText('Host 1')
@@ -1172,7 +1179,7 @@ describe('Allowed Hosts field', () => {
11721179
await userEvent.click(
11731180
screen.getByRole('button', { name: /install server/i })
11741181
)
1175-
expect(screen.queryByText(/invalid host/i)).not.toBeInTheDocument()
1182+
expect(screen.queryByText(/invalid host format/i)).not.toBeInTheDocument()
11761183
}
11771184
})
11781185

renderer/src/features/registry-servers/components/dynamic-array-field.tsx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ interface DynamicArrayFieldProps<
3232
inputLabelPrefix?: string
3333
tooltipContent?: string
3434
addButtonText?: string
35+
type?: 'text' | 'number'
3536
inputProps?: React.InputHTMLAttributes<HTMLInputElement>
3637
}
3738

@@ -42,6 +43,7 @@ export function DynamicArrayField<TFieldValues extends FieldValues>({
4243
tooltipContent,
4344
inputLabelPrefix = 'Item',
4445
addButtonText = 'Add',
46+
type = 'text',
4547
inputProps = {},
4648
}: DynamicArrayFieldProps<TFieldValues>) {
4749
const { fields, append, remove } = useFieldArray<
@@ -81,6 +83,7 @@ export function DynamicArrayField<TFieldValues extends FieldValues>({
8183
<FormControl className="w-full">
8284
<Input
8385
{...field}
86+
type={type}
8487
id={`${name}-${idx}`}
8588
aria-label={`${inputLabelPrefix} ${idx + 1}`}
8689
className="min-w-0 grow"

renderer/src/features/registry-servers/components/form-run-from-registry/index.tsx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,8 @@ export function FormRunFromRegistry({
111111
) || [],
112112
allowedHosts: server?.permissions?.network?.outbound?.allow_host || [],
113113
},
114+
reValidateMode: 'onChange',
115+
mode: 'onChange',
114116
})
115117

116118
const onSubmitForm = (data: FormSchemaRunFromRegistry) => {

renderer/src/features/registry-servers/components/form-run-from-registry/network-isolation-tab-content.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,11 +72,12 @@ export function NetworkIsolationTabContent({
7272
<DynamicArrayField<FormSchemaRunFromRegistry>
7373
/*
7474
// @ts-expect-error no time to fix this */
75-
name={'allowedPorts'}
75+
name="allowedPorts"
7676
label="Allowed ports"
7777
control={form.control}
7878
inputLabelPrefix="Port"
7979
addButtonText="Add a port"
80+
type="number"
8081
/>
8182
)}
8283
/>

renderer/src/features/registry-servers/lib/get-form-schema-run-from-registry.ts

Lines changed: 12 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -39,20 +39,6 @@ function refineEnvVar(
3939
return true
4040
}
4141

42-
const validatePort = (val: string) => {
43-
if (!val.trim()) return 'Port is required'
44-
if (!/^[0-9]+$/.test(val)) return 'Port must be a number'
45-
const num = Number(val)
46-
if (isNaN(num) || num < 1 || num > 65535) return 'Port must be 1-65535'
47-
return null
48-
}
49-
50-
const validateHost = (val: string) => {
51-
if (!val.trim()) return 'Host is required'
52-
if (!/^\.?([a-zA-Z0-9-]+\.)+[a-zA-Z]{2,}$/.test(val)) return 'Invalid host'
53-
return null
54-
}
55-
5642
/**
5743
* Returns the form schema used to validate the "run from registry" form.
5844
* The schema is dynamically generated based on the server's environment variables.
@@ -98,15 +84,22 @@ export function getFormSchemaRunFromRegistry({
9884
networkIsolation: z.boolean(),
9985
allowedHosts: z
10086
.string()
101-
.refine((val) => validateHost(val) === null, {
102-
message: 'Invalid host',
87+
.refine((val) => /^\.?([a-zA-Z0-9-]+\.)+[a-zA-Z]{2,}$/.test(val), {
88+
message: 'Invalid host format',
10389
})
10490
.array(),
10591
allowedPorts: z
10692
.string()
107-
.refine((val) => validatePort(val.toString()) === null, {
108-
message: 'Invalid port',
109-
})
93+
94+
.refine(
95+
(val) => {
96+
const num = parseInt(val, 10)
97+
return !isNaN(num) && num >= 1 && num <= 65535
98+
},
99+
{
100+
message: 'Port must be a number between 1 and 65535',
101+
}
102+
)
110103
.array(),
111104
})
112105
}

0 commit comments

Comments
 (0)