Skip to content

Commit daefd32

Browse files
committed
Fix no-proxy list: guard StringList event and validate entries
The StringList @change handler in WslProxy.vue lacked the Array.isArray guard added to ContainerEngineAllowedImages.vue during the Vue 3 update, causing non-array values to corrupt the noproxy setting. Also add validation that noproxy entries are IP addresses or CIDR subnets, since the iptables-based bypass mechanism cannot handle hostnames or wildcards. Fixes #9803 Signed-off-by: Jan Dubois <jan.dubois@suse.com>
1 parent 7df97ed commit daefd32

File tree

3 files changed

+166
-2
lines changed

3 files changed

+166
-2
lines changed

pkg/rancher-desktop/components/Preferences/WslProxy.vue

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ export default defineComponent({
123123
:items="preferences.experimental.virtualMachine.proxy.noproxy"
124124
:error-messages="noproxyErrorMessages"
125125
bulk-addition-delimiter=","
126-
@change="onChange('experimental.virtualMachine.proxy.noproxy', $event)"
126+
@change="Array.isArray($event) && onChange('experimental.virtualMachine.proxy.noproxy', $event)"
127127
@type:item="onType($event)"
128128
@errors="onDuplicate($event.duplicate)"
129129
/>

pkg/rancher-desktop/main/commandServer/__tests__/settingsValidator.spec.ts

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -993,4 +993,113 @@ describe('SettingsValidator', () => {
993993
);
994994
});
995995
});
996+
997+
describe('noproxy', () => {
998+
const fqname = 'experimental.virtualMachine.proxy.noproxy';
999+
1000+
function noproxySetting(noproxy: string[]): RecursivePartial<settings.Settings> {
1001+
return { experimental: { virtualMachine: { proxy: { noproxy } } } };
1002+
}
1003+
1004+
beforeEach(() => {
1005+
modules.os.platform.mockReturnValue('win32');
1006+
});
1007+
1008+
it('should accept valid IPv4 addresses', () => {
1009+
const [needToUpdate, errors] = subject.validateSettings(cfg, noproxySetting(['192.168.1.1']));
1010+
1011+
expect({ needToUpdate, errors }).toEqual({ needToUpdate: true, errors: [] });
1012+
});
1013+
1014+
it('should accept valid IPv4 CIDR subnets', () => {
1015+
const [needToUpdate, errors] = subject.validateSettings(cfg, noproxySetting(['10.0.0.0/8', '172.16.0.0/12']));
1016+
1017+
expect({ needToUpdate, errors }).toEqual({ needToUpdate: true, errors: [] });
1018+
});
1019+
1020+
it('should accept valid IPv6 addresses', () => {
1021+
const [needToUpdate, errors] = subject.validateSettings(cfg, noproxySetting(['::1', 'fe80::1']));
1022+
1023+
expect({ needToUpdate, errors }).toEqual({ needToUpdate: true, errors: [] });
1024+
});
1025+
1026+
it('should accept valid IPv6 CIDR subnets', () => {
1027+
const [needToUpdate, errors] = subject.validateSettings(cfg, noproxySetting(['fd00::/8', '::1/128']));
1028+
1029+
expect({ needToUpdate, errors }).toEqual({ needToUpdate: true, errors: [] });
1030+
});
1031+
1032+
it('should accept the default noproxy list unchanged without errors', () => {
1033+
const [needToUpdate, errors] = subject.validateSettings(cfg, noproxySetting(cfg.experimental.virtualMachine.proxy.noproxy));
1034+
1035+
expect({ needToUpdate, errors }).toEqual({ needToUpdate: false, errors: [] });
1036+
});
1037+
1038+
it('should reject hostnames', () => {
1039+
const [needToUpdate, errors] = subject.validateSettings(cfg, noproxySetting(['example.com']));
1040+
1041+
expect(needToUpdate).toBe(false);
1042+
expect(errors).toHaveLength(1);
1043+
expect(errors[0]).toContain('invalid entries');
1044+
expect(errors[0]).toContain('example.com');
1045+
});
1046+
1047+
it('should reject wildcard DNS entries', () => {
1048+
const [needToUpdate, errors] = subject.validateSettings(cfg, noproxySetting(['*.example.com']));
1049+
1050+
expect(needToUpdate).toBe(false);
1051+
expect(errors).toHaveLength(1);
1052+
expect(errors[0]).toContain('invalid entries');
1053+
expect(errors[0]).toContain('*.example.com');
1054+
});
1055+
1056+
it('should reject duplicate entries', () => {
1057+
const [needToUpdate, errors] = subject.validateSettings(cfg, noproxySetting(['10.0.0.1', '10.0.0.1']));
1058+
1059+
expect(needToUpdate).toBe(false);
1060+
expect(errors).toHaveLength(1);
1061+
expect(errors[0]).toContain('duplicate');
1062+
});
1063+
1064+
it('should reject non-array values', () => {
1065+
const [needToUpdate, errors] = subject.validateSettings(cfg, { experimental: { virtualMachine: { proxy: { noproxy: 'not-an-array' as any } } } });
1066+
1067+
expect(needToUpdate).toBe(false);
1068+
expect(errors).toHaveLength(1);
1069+
});
1070+
1071+
it('should reject CIDR with out-of-range prefix length', () => {
1072+
const [needToUpdate, errors] = subject.validateSettings(cfg, noproxySetting(['10.0.0.0/33']));
1073+
1074+
expect(needToUpdate).toBe(false);
1075+
expect(errors).toHaveLength(1);
1076+
expect(errors[0]).toContain('invalid entries');
1077+
});
1078+
1079+
it('should reject CIDR with non-numeric prefix', () => {
1080+
const [needToUpdate, errors] = subject.validateSettings(cfg, noproxySetting(['10.0.0.0/abc']));
1081+
1082+
expect(needToUpdate).toBe(false);
1083+
expect(errors).toHaveLength(1);
1084+
expect(errors[0]).toContain('invalid entries');
1085+
});
1086+
1087+
it('should report all invalid entries at once', () => {
1088+
const [needToUpdate, errors] = subject.validateSettings(cfg, noproxySetting(['foo.bar', '10.0.0.0/8', 'baz.qux']));
1089+
1090+
expect(needToUpdate).toBe(false);
1091+
expect(errors).toHaveLength(1);
1092+
expect(errors[0]).toContain('foo.bar');
1093+
expect(errors[0]).toContain('baz.qux');
1094+
});
1095+
1096+
it('should be gated to win32 platform', () => {
1097+
modules.os.platform.mockReturnValue('darwin');
1098+
const [needToUpdate, errors] = subject.validateSettings(cfg, noproxySetting(['10.0.0.1']));
1099+
1100+
expect(needToUpdate).toBe(false);
1101+
expect(errors).toHaveLength(1);
1102+
expect(errors[0]).toContain('isn\'t supported');
1103+
});
1104+
});
9961105
});

pkg/rancher-desktop/main/commandServer/settingsValidator.ts

Lines changed: 56 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import net from 'net';
12
import os from 'os';
23

34
import _ from 'lodash';
@@ -148,7 +149,7 @@ export default class SettingsValidator {
148149
password: this.checkPlatform('win32', this.checkString),
149150
port: this.checkPlatform('win32', this.checkNumber(1, 65535)),
150151
username: this.checkPlatform('win32', this.checkString),
151-
noproxy: this.checkPlatform('win32', this.checkUniqueStringArray),
152+
noproxy: this.checkPlatform('win32', this.checkNoproxyList),
152153
},
153154
sshPortForwarder: this.checkLima(this.checkBoolean),
154155
},
@@ -663,6 +664,60 @@ export default class SettingsValidator {
663664
return Array.from(duplicates).concat(whiteSpaceMembers);
664665
}
665666

667+
/**
668+
* Validate that a string is an IP address or CIDR subnet.
669+
* Accepts IPv4 and IPv6 addresses with optional prefix length.
670+
*/
671+
protected static isIPOrCIDR(entry: string): boolean {
672+
const slashIndex = entry.indexOf('/');
673+
674+
if (slashIndex === -1) {
675+
return net.isIP(entry) !== 0;
676+
}
677+
const address = entry.substring(0, slashIndex);
678+
const prefixStr = entry.substring(slashIndex + 1);
679+
const ipVersion = net.isIP(address);
680+
681+
if (ipVersion === 0) {
682+
return false;
683+
}
684+
if (!/^\d+$/.test(prefixStr)) {
685+
return false;
686+
}
687+
const prefix = parseInt(prefixStr, 10);
688+
const maxPrefix = ipVersion === 4 ? 32 : 128;
689+
690+
return prefix >= 0 && prefix <= maxPrefix;
691+
}
692+
693+
/**
694+
* Validate the noproxy list: must be unique strings, each a valid IP or CIDR.
695+
*/
696+
protected checkNoproxyList<S>(mergedSettings: S, currentValue: string[], desiredValue: string[], errors: string[], fqname: string): boolean {
697+
if (!Array.isArray(desiredValue) || desiredValue.some(s => typeof (s) !== 'string')) {
698+
errors.push(this.invalidSettingMessage(fqname, desiredValue));
699+
700+
return false;
701+
}
702+
const duplicateValues = this.findDuplicates(desiredValue);
703+
704+
if (duplicateValues.length > 0) {
705+
duplicateValues.sort(Intl.Collator().compare);
706+
errors.push(`field "${ fqname }" has duplicate entries: "${ duplicateValues.join('", "') }"`);
707+
708+
return false;
709+
}
710+
const invalidEntries = desiredValue.filter(entry => !SettingsValidator.isIPOrCIDR(entry));
711+
712+
if (invalidEntries.length > 0) {
713+
errors.push(`field "${ fqname }" has invalid entries (must be IP addresses or CIDR subnets): "${ invalidEntries.join('", "') }"`);
714+
715+
return false;
716+
}
717+
718+
return currentValue.length !== desiredValue.length || currentValue.some((v, i) => v !== desiredValue[i]);
719+
}
720+
666721
protected checkInstalledExtensions(
667722
mergedSettings: Settings,
668723
currentValue: Record<string, string>,

0 commit comments

Comments
 (0)