mirror of
https://github.com/n8n-io/n8n.git
synced 2026-08-30 18:01:23 +08:00
fix(core): Normalize env values before schema-based parsing (backport to release-candidate/2.35.x) (#36732)
Co-authored-by: Mike Repeć <mike.repec@n8n.io> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: Danny Martini <danny@n8n.io>
This commit is contained in:
committed by
GitHub
parent
eb85ccde78
commit
0664c7db29
@@ -19,16 +19,18 @@ describe('SecurityConfig', () => {
|
||||
expect(Container.get(SecurityConfig).awsSystemCredentialsSdkSources).toBe('all');
|
||||
});
|
||||
|
||||
// Leading/trailing whitespace is trimmed before parsing; inner whitespace is
|
||||
// preserved and handled by the consumer (`usesSdk` trims per source).
|
||||
test.each([
|
||||
'all',
|
||||
'none',
|
||||
'environment',
|
||||
'environment,instanceMetadata',
|
||||
' environment , podIdentity ',
|
||||
'environment,',
|
||||
])('accepts valid value %p', (value) => {
|
||||
['all', 'all'],
|
||||
['none', 'none'],
|
||||
['environment', 'environment'],
|
||||
['environment,instanceMetadata', 'environment,instanceMetadata'],
|
||||
[' environment , podIdentity ', 'environment , podIdentity'],
|
||||
['environment,', 'environment,'],
|
||||
])('accepts valid value %p', (value, expected) => {
|
||||
process.env = { N8N_AWS_SYSTEM_CREDENTIALS_SDK_SOURCES: value };
|
||||
expect(Container.get(SecurityConfig).awsSystemCredentialsSdkSources).toBe(value);
|
||||
expect(Container.get(SecurityConfig).awsSystemCredentialsSdkSources).toBe(expected);
|
||||
});
|
||||
|
||||
test('falls back to the default and warns on an unknown source', () => {
|
||||
|
||||
@@ -23,17 +23,23 @@ const readEnv = (envName: string) => {
|
||||
const filePath = process.env[`${envName}_FILE`];
|
||||
if (filePath) {
|
||||
const value = readFileSync(filePath, 'utf8');
|
||||
if (value !== value.trim()) {
|
||||
// File contents commonly carry a trailing newline (e.g. `echo value > file`)
|
||||
const trimmed = value.trim();
|
||||
if (value !== trimmed) {
|
||||
console.warn(
|
||||
`[n8n] Warning: The file specified by ${envName}_FILE contains leading or trailing whitespace, which may cause authentication failures.`,
|
||||
`[n8n] Warning: The file specified by ${envName}_FILE contained leading or trailing whitespace; the value was trimmed.`,
|
||||
);
|
||||
}
|
||||
return value;
|
||||
return trimmed;
|
||||
}
|
||||
|
||||
return undefined;
|
||||
};
|
||||
|
||||
// Env values commonly carry stray whitespace or surrounding quotes (e.g. compose env_file,
|
||||
// `echo value > file`); strip both so parsing doesn't silently fall back to the default value.
|
||||
const normalizeEnvValue = (value: string) => value.trim().replace(/^(['"])(.*)\1$/, '$2');
|
||||
|
||||
export const Config: ClassDecorator = (ConfigClass: Class) => {
|
||||
const factory = function (...args: unknown[]) {
|
||||
const config = new (ConfigClass as new (...a: unknown[]) => Record<PropertyKey, unknown>)(
|
||||
@@ -52,7 +58,7 @@ export const Config: ClassDecorator = (ConfigClass: Class) => {
|
||||
if (value === undefined) continue;
|
||||
|
||||
if (schema) {
|
||||
const result = schema.safeParse(value);
|
||||
const result = schema.safeParse(normalizeEnvValue(value));
|
||||
if (result.error) {
|
||||
console.warn(
|
||||
`Invalid value for ${envName} - ${result.error.issues[0].message}. Falling back to default value.`,
|
||||
@@ -83,7 +89,7 @@ export const Config: ClassDecorator = (ConfigClass: Class) => {
|
||||
config[key] = new Date(timestamp);
|
||||
}
|
||||
} else if (type === String) {
|
||||
config[key] = value.trim().replace(/^(['"])(.*)\1$/, '$2');
|
||||
config[key] = normalizeEnvValue(value);
|
||||
} else {
|
||||
config[key] = new (type as Constructable)(value);
|
||||
}
|
||||
|
||||
@@ -872,7 +872,7 @@ describe('GlobalConfig', () => {
|
||||
expect(config.database.postgresdb.password).toBe('password-from-file');
|
||||
expect(consoleWarnMock).toHaveBeenCalledWith(
|
||||
expect.stringContaining(
|
||||
'DB_POSTGRESDB_PASSWORD_FILE contains leading or trailing whitespace',
|
||||
'DB_POSTGRESDB_PASSWORD_FILE contained leading or trailing whitespace',
|
||||
),
|
||||
);
|
||||
});
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { Container } from '@n8n/di';
|
||||
import fs from 'fs';
|
||||
import { z } from 'zod';
|
||||
|
||||
import { Config, Env } from '../src/decorators';
|
||||
|
||||
@@ -63,7 +64,7 @@ describe('decorators', () => {
|
||||
const config = Container.get(TestConfig);
|
||||
expect(config.value).toBe('secret-value');
|
||||
expect(consoleWarnSpy).toHaveBeenCalledWith(
|
||||
expect.stringContaining('TEST_VALUE_FILE contains leading or trailing whitespace'),
|
||||
expect.stringContaining('TEST_VALUE_FILE contained leading or trailing whitespace'),
|
||||
);
|
||||
consoleWarnSpy.mockRestore();
|
||||
});
|
||||
@@ -83,4 +84,46 @@ describe('decorators', () => {
|
||||
expect(config.value).toBe('direct-value');
|
||||
expect(mockFs.readFileSync).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should trim whitespace from a direct env value before parsing it with a zod schema', () => {
|
||||
process.env.TEST_VALUE = 'legacy ';
|
||||
|
||||
@Config
|
||||
class TestConfig {
|
||||
@Env('TEST_VALUE', z.enum(['legacy', 'vm']))
|
||||
value: string = 'vm';
|
||||
}
|
||||
|
||||
expect(Container.get(TestConfig).value).toBe('legacy');
|
||||
});
|
||||
|
||||
it('should strip surrounding quotes from an env value before parsing it with a zod schema', () => {
|
||||
process.env.TEST_VALUE = "'legacy'";
|
||||
|
||||
@Config
|
||||
class TestConfig {
|
||||
@Env('TEST_VALUE', z.enum(['legacy', 'vm']))
|
||||
value: string = 'vm';
|
||||
}
|
||||
|
||||
expect(Container.get(TestConfig).value).toBe('legacy');
|
||||
});
|
||||
|
||||
it('should trim trailing newline from _FILE value before parsing it with a zod schema', () => {
|
||||
const filePath = '/path/to/secret';
|
||||
process.env.TEST_VALUE_FILE = filePath;
|
||||
mockFs.readFileSync.mockReturnValueOnce('legacy\n');
|
||||
const consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
|
||||
|
||||
@Config
|
||||
class TestConfig {
|
||||
@Env('TEST_VALUE', z.enum(['legacy', 'vm']))
|
||||
value: string = 'vm';
|
||||
}
|
||||
|
||||
const config = Container.get(TestConfig);
|
||||
expect(config.value).toBe('legacy');
|
||||
expect(consoleWarnSpy).toHaveBeenCalledWith(expect.stringContaining('the value was trimmed'));
|
||||
consoleWarnSpy.mockRestore();
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user