Skip to content

Validate Lambda deploy config #645

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ralyodio merged 1 commit into from
Jun 6, 2026
Merged

Validate Lambda deploy config #645

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions packages/targets/deploy-lambda/src/index.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,4 +185,31 @@ describe('AWS Lambda deployment target', () => {
functionName: '',
})).rejects.toThrow('functionName is required');
});

it('rejects invalid optional Lambda config before writing a plan', async () => {
await expect(adapter.build(fakeBuildContext() as any, {
functionName: 'my-function',
handler: ' ',
})).rejects.toThrow('deploy-lambda requires handler');

await expect(adapter.build(fakeBuildContext() as any, {
functionName: 'my-function',
memorySize: 64,
})).rejects.toThrow('memorySize must be an integer from 128 to 10240');

await expect(adapter.build(fakeBuildContext() as any, {
functionName: 'my-function',
timeout: 901,
})).rejects.toThrow('timeout must be an integer from 1 to 900');

await expect(adapter.build(fakeBuildContext() as any, {
functionName: 'my-function',
layers: [' '],
})).rejects.toThrow('deploy-lambda requires layers[0]');

await expect(adapter.build(fakeBuildContext() as any, {
functionName: 'my-function',
environment: { '1BAD': 'value' },
})).rejects.toThrow('environment variable "1BAD" must start with a letter');
});
});
60 changes: 58 additions & 2 deletions packages/targets/deploy-lambda/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,66 @@ function functionName(config: Config): string {
return fn;
}

function optionalText(value: string | undefined, field: string): string | undefined {
if (value === undefined) return undefined;
const text = value.trim();
if (!text) throw new Error(`deploy-lambda requires ${field}`);
return text;
}

function optionalInteger(value: number | undefined, field: string, min: number, max: number): number | undefined {
if (value === undefined) return undefined;
if (!Number.isInteger(value) || value < min || value > max) {
throw new Error(`deploy-lambda ${field} must be an integer from ${min} to ${max}`);
}
return value;
}

function environmentVariables(value: Record<string, string> | undefined): Record<string, string> | undefined {
if (value === undefined) return undefined;
const entries = Object.entries(value);
for (const [key, entryValue] of entries) {
if (!/^[A-Za-z][A-Za-z0-9_]+$/.test(key)) {

@greptile-apps greptile-apps Bot Jun 5, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 The environment variable key regex uses + (one-or-more) for the character class after the first letter, which means single-character names like X, A, or Z are always rejected even though they are fully valid POSIX and AWS Lambda environment variable names. Any caller passing { X: '1' } or a similar single-letter key gets an error that doesn't match the stated rule.

Suggested change
if (!/^[A-Za-z][A-Za-z0-9_]+$/.test(key)) {
if (!/^[A-Za-z][A-Za-z0-9_]*$/.test(key)) {

throw new Error(`deploy-lambda environment variable "${key}" must start with a letter and contain only letters, numbers, or underscores`);
}
if (typeof entryValue !== 'string') {
throw new Error(`deploy-lambda environment variable "${key}" must be a string`);
}
}
return value;
}

function layerArns(value: string[] | undefined): string[] | undefined {
return value?.map((layer, index) => optionalText(layer, `layers[${index}]`)!);
}

function normalizedConfig(config: Config): Config {
return {
...config,
functionName: functionName(config),
handler: optionalText(config.handler, 'handler'),
runtime: optionalText(config.runtime, 'runtime'),
role: optionalText(config.role, 'role'),
zipFile: optionalText(config.zipFile, 'zipFile'),
region: optionalText(config.region, 'region'),
description: optionalText(config.description, 'description'),
environment: environmentVariables(config.environment),
layers: layerArns(config.layers),
memorySize: optionalInteger(config.memorySize, 'memorySize', 128, 10240),
timeout: optionalInteger(config.timeout, 'timeout', 1, 900),
};
}

function region(ctx: { secret(key: string): string | undefined }, config: Config): string {
return config.region ?? ctx.secret('AWS_REGION') ?? 'us-east-1';
return optionalText(config.region, 'region') ?? optionalText(ctx.secret('AWS_REGION'), 'AWS_REGION') ?? 'us-east-1';
}
Comment on lines 77 to 79

@greptile-apps greptile-apps Bot Jun 5, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 optionalText wrapping ctx.secret('AWS_REGION') silently changes error behaviour: if the vault returns a blank string (""), the call throws "deploy-lambda requires AWS_REGION" instead of falling back to 'us-east-1'. An environment where the secret was stored as an empty string would now hard-error at every build / ship call rather than using the default region.


function zipFile(ctx: { outDir: string }, config: Config): string {
return config.zipFile ?? join(ctx.outDir, 'function.zip');
return optionalText(config.zipFile, 'zipFile') ?? join(ctx.outDir, 'function.zip');
}

function applyOptionalCreateArgs(args: string[], config: Config): string[] {
config = normalizedConfig(config);
Comment on lines 85 to +86

@greptile-apps greptile-apps Bot Jun 5, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 normalizedConfig is now called multiple times on the same execution path. All validators are idempotent so this causes no incorrect behaviour today, but each call redundantly re-validates every field. If a heavier side-effect is ever added to a validator, this chain would multiply its impact unexpectedly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

if (config.description) args.push('--description', config.description);
if (config.timeout !== undefined) args.push('--timeout', String(config.timeout));
if (config.memorySize !== undefined) args.push('--memory-size', String(config.memorySize));
Expand All @@ -45,6 +96,7 @@ function applyOptionalCreateArgs(args: string[], config: Config): string[] {
}

function updateArgs(config: Config, artifact: string, awsRegion: string): string[] {
config = normalizedConfig(config);
const args = [
'lambda',
'update-function-code',
Expand All @@ -60,6 +112,7 @@ function updateArgs(config: Config, artifact: string, awsRegion: string): string
}

function createArgs(config: Config, artifact: string, awsRegion: string, role: string): string[] {
config = normalizedConfig(config);
return applyOptionalCreateArgs([
'lambda',
'create-function',
Expand All @@ -82,6 +135,7 @@ function renderPlan(
ctx: { outDir: string; version: string; secret(key: string): string | undefined },
config: Config
): string {
config = normalizedConfig(config);
const artifact = zipFile(ctx, config);
const awsRegion = region(ctx, config);
const plannedRole = config.role ?? '<AWS_LAMBDA_ROLE>';
Expand Down Expand Up @@ -132,6 +186,7 @@ export default defineTarget<Config>({
label: 'AWS Lambda',

async build(ctx, config) {
config = normalizedConfig(config);
const fn = functionName(config);
const planPath = join(ctx.outDir, 'lambda-deploy.json');
ctx.log(`lambda plan - function=${fn} region=${region(ctx, config)}`);
Expand All @@ -141,6 +196,7 @@ export default defineTarget<Config>({
},

async ship(ctx, config) {
config = normalizedConfig(config);
const fn = functionName(config);
const awsRegion = region(ctx, config);
const artifact = zipFile(ctx, config);
Expand Down
Loading