From 760a9693e73ac24ec43d7fdab04eecabcf645ede Mon Sep 17 00:00:00 2001 From: sunlei Date: Sun, 26 Jul 2026 16:27:03 +0800 Subject: [PATCH] =?UTF-8?q?fix:=20=E4=B8=A5=E6=A0=BC=E6=A0=A1=E9=AA=8CTCP?= =?UTF-8?q?=E5=8F=91=E5=B8=83=E5=8A=A8=E4=BD=9C?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../network-tcp-release-policy.service.ts | 174 ++++++++++++----- ...network-tcp-release-policy.service.spec.ts | 175 ++++++++++++++++++ 2 files changed, 302 insertions(+), 47 deletions(-) diff --git a/src/modules/admin/platform-config/network-management/network-tcp-release-policy.service.ts b/src/modules/admin/platform-config/network-management/network-tcp-release-policy.service.ts index 044853b..c3fab38 100644 --- a/src/modules/admin/platform-config/network-management/network-tcp-release-policy.service.ts +++ b/src/modules/admin/platform-config/network-management/network-tcp-release-policy.service.ts @@ -30,6 +30,14 @@ export type TcpReleaseMutation = export class NetworkTcpReleasePolicyError extends Error {} +const INVALID_MUTATION_MESSAGE = 'Invalid TCP NATMap mutation'; +const RELEASE_STATE_KEYS = [ + 'externalPort', + 'internalPort', + 'natmapDesiredEnabled', + 'protocolMode', +] as const; + @Injectable() export class NetworkTcpReleasePolicyService { constructor(private readonly configService: ConfigService) {} @@ -102,28 +110,64 @@ export class NetworkTcpReleasePolicyService { } private assertMutationShape(mutation: TcpReleaseMutation): void { - const value = mutation as Partial<{ - after: TcpReleaseState; - before: TcpReleaseState; - current: TcpReleaseState; - kind: TcpReleaseMutation['kind']; - }>; + const value = this.record(mutation); switch (value.kind) { - case 'create': - if (value.after) return; - break; - case 'update': - case 'natmap-disable': - case 'protocol-shrink': - if (value.before && value.after) return; - break; + case 'create': { + this.assertExactKeys(value, ['after', 'kind']); + this.releaseState(value.after); + return; + } + case 'update': { + this.assertExactKeys(value, ['after', 'before', 'kind']); + this.releaseState(value.before); + this.releaseState(value.after); + return; + } case 'retry': - case 'natmap-enable': - case 'delete': - if (value.current) return; - break; + case 'delete': { + this.assertExactKeys(value, ['current', 'kind']); + this.releaseState(value.current); + return; + } + case 'natmap-enable': { + this.assertExactKeys(value, ['current', 'kind']); + const current = this.releaseState(value.current); + if (!this.hasTcp(current) || !current.natmapDesiredEnabled) { + this.invalidMutation(); + } + return; + } + case 'natmap-disable': { + this.assertExactKeys(value, ['after', 'before', 'kind']); + const before = this.releaseState(value.before); + const after = this.releaseState(value.after); + if ( + !this.hasTcp(before) || + before.protocolMode !== after.protocolMode || + !this.portsUnchanged(before, after) || + !before.natmapDesiredEnabled || + after.natmapDesiredEnabled + ) { + this.invalidMutation(); + } + return; + } + case 'protocol-shrink': { + this.assertExactKeys(value, ['after', 'before', 'kind']); + const before = this.releaseState(value.before); + const after = this.releaseState(value.after); + if ( + before.protocolMode !== 'tcp_udp' || + after.protocolMode !== 'udp' || + !this.portsUnchanged(before, after) || + after.natmapDesiredEnabled + ) { + this.invalidMutation(); + } + return; + } } - throw new NetworkTcpReleasePolicyError('Invalid TCP NATMap mutation'); + this.invalidMutation(); } private touchesTcp(mutation: TcpReleaseMutation): boolean { @@ -138,34 +182,10 @@ export class NetworkTcpReleasePolicyService { if (mutation.kind === 'delete') { return mutation.current.protocolMode === 'tcp'; } - if (mutation.kind === 'natmap-disable') { - const valid = - this.hasTcp(mutation.before) && - mutation.before.protocolMode === mutation.after.protocolMode && - this.portsUnchanged(mutation.before, mutation.after) && - mutation.before.natmapDesiredEnabled && - !mutation.after.natmapDesiredEnabled; - if (!valid) { - throw new NetworkTcpReleasePolicyError( - 'Invalid TCP NATMap disable mutation', - ); - } - return true; - } - if (mutation.kind === 'protocol-shrink') { - const valid = - mutation.before.protocolMode === 'tcp_udp' && - mutation.after.protocolMode === 'udp' && - this.portsUnchanged(mutation.before, mutation.after) && - !mutation.after.natmapDesiredEnabled; - if (!valid) { - throw new NetworkTcpReleasePolicyError( - 'Invalid TCP NATMap protocol shrink', - ); - } - return true; - } - return false; + return ( + mutation.kind === 'natmap-disable' || + mutation.kind === 'protocol-shrink' + ); } private mutationPort(mutation: TcpReleaseMutation): number { @@ -183,6 +203,66 @@ export class NetworkTcpReleasePolicyService { ); } + private record(value: unknown): Record { + if (typeof value !== 'object' || value === null || Array.isArray(value)) { + this.invalidMutation(); + } + return value as Record; + } + + private assertExactKeys( + value: Record, + expected: readonly string[], + ): void { + const expectedKeys = new Set(expected); + const actualKeys = Reflect.ownKeys(value); + if ( + actualKeys.length !== expected.length || + actualKeys.some( + (key) => typeof key !== 'string' || !expectedKeys.has(key), + ) || + expected.some( + (key) => !Object.prototype.hasOwnProperty.call(value, key), + ) + ) { + this.invalidMutation(); + } + } + + private releaseState(value: unknown): TcpReleaseState { + const state = this.record(value); + this.assertExactKeys(state, RELEASE_STATE_KEYS); + const protocolMode = state.protocolMode; + const externalPort = state.externalPort; + const internalPort = state.internalPort; + const natmapDesiredEnabled = state.natmapDesiredEnabled; + if ( + (protocolMode !== 'tcp' && + protocolMode !== 'tcp_udp' && + protocolMode !== 'udp') || + !this.isPort(externalPort) || + !this.isPort(internalPort) || + typeof natmapDesiredEnabled !== 'boolean' || + (protocolMode === 'udp' && natmapDesiredEnabled) + ) { + this.invalidMutation(); + } + return state as TcpReleaseState; + } + + private isPort(value: unknown): value is number { + return ( + typeof value === 'number' && + Number.isInteger(value) && + value >= 1 && + value <= 65_535 + ); + } + + private invalidMutation(): never { + throw new NetworkTcpReleasePolicyError(INVALID_MUTATION_MESSAGE); + } + private invalidCanaryPorts(): never { throw new NetworkTcpReleasePolicyError('Invalid TCP NATMap canary ports'); } diff --git a/test/admin/network-management/network-tcp-release-policy.service.spec.ts b/test/admin/network-management/network-tcp-release-policy.service.spec.ts index 26e1781..3d6bedd 100644 --- a/test/admin/network-management/network-tcp-release-policy.service.spec.ts +++ b/test/admin/network-management/network-tcp-release-policy.service.spec.ts @@ -26,6 +26,63 @@ const udp = (overrides = {}) => ({ ...overrides, }); +const INVALID_MUTATION_MESSAGE = 'Invalid TCP NATMap mutation'; +const validMutationCases = [ + { + kind: 'create', + mutation: { after: tcp(), kind: 'create' }, + stateKeys: ['after'], + }, + { + kind: 'update', + mutation: { after: tcp(), before: tcp(), kind: 'update' }, + stateKeys: ['after', 'before'], + }, + { + kind: 'retry', + mutation: { current: tcp(), kind: 'retry' }, + stateKeys: ['current'], + }, + { + kind: 'natmap-enable', + mutation: { current: tcp(), kind: 'natmap-enable' }, + stateKeys: ['current'], + }, + { + kind: 'natmap-disable', + mutation: { + after: tcp({ natmapDesiredEnabled: false }), + before: tcp(), + kind: 'natmap-disable', + }, + stateKeys: ['after', 'before'], + }, + { + kind: 'protocol-shrink', + mutation: { + after: udp({ externalPort: 48213, internalPort: 48213 }), + before: tcp({ protocolMode: 'tcp_udp' }), + kind: 'protocol-shrink', + }, + stateKeys: ['after', 'before'], + }, + { + kind: 'delete', + mutation: { current: tcp(), kind: 'delete' }, + stateKeys: ['current'], + }, +] as const; + +function expectInvalidMutation(mutation: unknown): void { + const service = policy({ + NETWORK_TCP_NATMAP_CANARY_PORTS: 'not-a-port', + NETWORK_TCP_NATMAP_RELEASE_MODE: 'not-a-mode', + }); + const validate = () => service.assertMutationAllowed(mutation as never); + expect(validate).toThrow(NetworkTcpReleasePolicyError); + expect(validate).toThrow(INVALID_MUTATION_MESSAGE); +} + describe('NetworkTcpReleasePolicyService', () => { it('fails closed by default and parses the four release modes', () => { expect(policy({}).readMode()).toBe('off'); @@ -198,4 +255,122 @@ describe('NetworkTcpReleasePolicyService', () => { service.assertMutationAllowed({ current: tcp(), kind: 'retry' }), ).toThrow(NetworkTcpReleasePolicyError); }); + + it.each(validMutationCases)( + 'accepts the exact valid $kind mutation record', + ({ mutation }) => { + expect(() => + policy({ + NETWORK_TCP_NATMAP_RELEASE_MODE: 'on', + }).assertMutationAllowed(mutation), + ).not.toThrow(); + }, + ); + + it.each(validMutationCases)( + 'rejects malformed top-level and nested records for $kind before reading release config', + ({ mutation, stateKeys }) => { + const record = mutation as unknown as Record; + expectInvalidMutation({ ...record, unexpected: true }); + expectInvalidMutation( + Object.fromEntries( + Object.entries(record).filter(([key]) => key !== 'kind'), + ), + ); + + for (const stateKey of stateKeys) { + expectInvalidMutation( + Object.fromEntries( + Object.entries(record).filter(([key]) => key !== stateKey), + ), + ); + for (const invalidState of [null, [], {}, 'tcp', 1]) { + expectInvalidMutation({ ...record, [stateKey]: invalidState }); + } + expectInvalidMutation({ + ...record, + [stateKey]: { + ...(record[stateKey] as Record), + unexpected: true, + }, + }); + } + }, + ); + + it.each([ + ['missing protocol', { ...tcp(), protocolMode: undefined }], + ['unknown protocol', { ...tcp(), protocolMode: 'icmp' }], + ['missing external port', { ...tcp(), externalPort: undefined }], + ['NaN external port', { ...tcp(), externalPort: Number.NaN }], + ['fractional external port', { ...tcp(), externalPort: 48213.5 }], + ['zero external port', { ...tcp(), externalPort: 0 }], + ['out-of-range external port', { ...tcp(), externalPort: 65_536 }], + ['string external port', { ...tcp(), externalPort: '48213' }], + ['missing internal port', { ...tcp(), internalPort: undefined }], + ['NaN internal port', { ...tcp(), internalPort: Number.NaN }], + ['fractional internal port', { ...tcp(), internalPort: 48213.5 }], + ['zero internal port', { ...tcp(), internalPort: 0 }], + ['out-of-range internal port', { ...tcp(), internalPort: 65_536 }], + ['string internal port', { ...tcp(), internalPort: '48213' }], + [ + 'missing NATMap intent', + { ...tcp(), natmapDesiredEnabled: undefined }, + ], + ['non-boolean NATMap intent', { ...tcp(), natmapDesiredEnabled: 1 }], + ['UDP with NATMap enabled', { ...udp(), natmapDesiredEnabled: true }], + ])('rejects a state with %s', (_label, after) => { + expectInvalidMutation({ after, kind: 'create' }); + }); + + it.each([ + [ + 'NATMap enable on UDP', + { current: udp(), kind: 'natmap-enable' }, + ], + [ + 'NATMap enable without enabled intent', + { + current: tcp({ natmapDesiredEnabled: false }), + kind: 'natmap-enable', + }, + ], + [ + 'NATMap disable on UDP', + { + after: udp(), + before: udp(), + kind: 'natmap-disable', + }, + ], + [ + 'NATMap disable without an intent transition', + { + after: tcp(), + before: tcp(), + kind: 'natmap-disable', + }, + ], + [ + 'protocol shrink from TCP-only', + { + after: udp({ externalPort: 48213, internalPort: 48213 }), + before: tcp(), + kind: 'protocol-shrink', + }, + ], + ])('rejects the impossible %s mutation', (_label, mutation) => { + expectInvalidMutation(mutation); + }); + + it.each([ + ['null', null], + ['array', []], + ['string', 'create'], + ['number', 1], + ['empty object', {}], + ['unknown kind', { after: tcp(), kind: 'replace' }], + ])('rejects a %s mutation container', (_label, mutation) => { + expectInvalidMutation(mutation); + }); });