added support for vault keys usage inside NodeJS code component #88
Conversation
WalkthroughEnvironment-variable awareness was added to AWS Lambda code generation, hashing, and deployment. Hashing now includes env values. Code generation embeds env handling. Deployment fetches and applies env vars, updating function configuration when needed. New helpers resolve vault-backed env vars and sanitize vault placeholders before validation and hashing. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant CM as ComputeManager (AWSLambdaCode.class)
participant H as Helpers (AWSLambdaCode.helper)
participant VAULT as Vault
participant AWS as AWS Lambda
CM->>H: getCurrentEnvironmentVariables(teamId, code)
H->>VAULT: fetch secrets for {{KEY(NAME)}} placeholders
VAULT-->>H: envVariables map
H-->>CM: envVariables
CM->>H: generateLambdaCode(code, parameters, envVariables)
H-->>CM: env-aware code bundle
CM->>H: generateCodeHash(code, inputs, sorted(env values))
H-->>CM: codeHash
CM->>H: createOrUpdateLambdaFunction(name, zipPath, awsConfigs, envVariables)
alt function exists
H->>AWS: UpdateFunctionConfiguration(envVariables)
AWS-->>H: updated config
else create new
H->>AWS: CreateFunction(envVariables)
AWS-->>H: function created
end
CM-->>CM: verify deployment by codeHash
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
packages/core/src/helpers/AWSLambdaCode.helper.ts (2)
87-90: Unused parameter in replaceVaultKeysTemplateVarsThe
envVariablesparameter is not used in the function. If it's not needed, consider removing it to avoid confusion.-function replaceVaultKeysTemplateVars(code: string, envVariables: Record<string, string>) { +function replaceVaultKeysTemplateVars(code: string) { const regex = /\{\{KEY\((.*?)\)\}\}/g; return code.replaceAll(regex, (match, p1) => `process.env.${p1}`); }Then update the call sites:
- const code = replaceVaultKeysTemplateVars(rawCode, {}); + const code = replaceVaultKeysTemplateVars(rawCode);
92-95: Consider simplifying the conditional logicSince
replaceVaultKeysTemplateVarsdoesn't use theenvVariablesparameter, the conditional check seems unnecessary.If you remove the unused parameter as suggested earlier:
- const codeWithEnvVariables = envVariables && Object.keys(envVariables).length ? replaceVaultKeysTemplateVars(code, envVariables) : code; + const codeWithEnvVariables = replaceVaultKeysTemplateVars(code);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (2)
packages/core/src/helpers/AWSLambdaCode.helper.ts(12 hunks)packages/core/src/subsystems/ComputeManager/Code.service/connectors/AWSLambdaCode.class.ts(5 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
packages/core/src/subsystems/ComputeManager/Code.service/connectors/AWSLambdaCode.class.ts (1)
packages/core/src/helpers/AWSLambdaCode.helper.ts (7)
getDeployedFunction(314-337)getDeployedCodeHash(74-78)getCurrentEnvironmentVariables(575-583)getSortedObjectValues(585-588)generateCodeHash(23-28)generateLambdaCode(92-114)createOrUpdateLambdaFunction(129-213)
packages/core/src/helpers/AWSLambdaCode.helper.ts (1)
packages/core/src/subsystems/Security/Vault.service/Vault.helper.ts (1)
VaultHelper(7-30)
🔇 Additional comments (12)
packages/core/src/subsystems/ComputeManager/Code.service/connectors/AWSLambdaCode.class.ts (3)
8-8: LGTM!The new imports are properly added to support environment variable handling.
40-43: Good implementation of deterministic hashing!The sorting of environment variable values ensures consistent hash generation, which is crucial for detecting changes in environment configuration.
62-62: Proper propagation of environment variables!Environment variables are correctly passed to both code generation and Lambda function creation/update.
Also applies to: 73-73
packages/core/src/helpers/AWSLambdaCode.helper.ts (9)
5-5: LGTM!The UpdateFunctionConfigurationCommand import is properly added for environment variable configuration updates.
23-28: Proper hash generation with environment variables!The inclusion of environment variables in the hash ensures Lambda functions are redeployed when environment configuration changes.
30-32: Smart sanitization approach!Replacing vault key templates before hashing ensures consistent hash generation regardless of the actual secret values.
155-158: Excellent environment variable handling!The function properly manages environment variables for both Lambda function updates and creation, with appropriate deployment status verification.
Also applies to: 202-202
215-224: Clean implementation of configuration update!The function properly updates Lambda environment variables using the AWS SDK.
382-385: Smart validation approach!Sanitizing vault key templates before AST parsing prevents validation errors from the template syntax.
553-563: Robust key extraction implementation!The function correctly extracts all vault key names from the template syntax.
575-583: Efficient parallel fetching of vault secrets!The function properly fetches all required vault secrets in parallel and aggregates them into environment variables.
585-588: Good implementation for deterministic ordering!The function ensures consistent ordering of object values, which is essential for reproducible hash generation.
| export function extractAllKeyNamesFromTemplateVars(input: string): string[] { | ||
| const regex = /\{\{KEY\((.*?)\)\}\}/g; | ||
| const matches = []; | ||
| let match; | ||
| while ((match = regex.exec(input)) !== null) { | ||
| if (match[1]) { | ||
| matches.push(match[1]); | ||
| } | ||
| } | ||
| return matches; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add validation for vault key names
Consider validating that extracted key names are valid environment variable names to prevent issues with special characters.
export function extractAllKeyNamesFromTemplateVars(input: string): string[] {
const regex = /\{\{KEY\((.*?)\)\}\}/g;
const matches = [];
let match;
while ((match = regex.exec(input)) !== null) {
if (match[1]) {
+ // Validate key name contains only alphanumeric and underscore
+ if (!/^[a-zA-Z_][a-zA-Z0-9_]*$/.test(match[1])) {
+ throw new Error(`Invalid vault key name: "${match[1]}". Key names must be valid environment variable names.`);
+ }
matches.push(match[1]);
}
}
return matches;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function extractAllKeyNamesFromTemplateVars(input: string): string[] { | |
| const regex = /\{\{KEY\((.*?)\)\}\}/g; | |
| const matches = []; | |
| let match; | |
| while ((match = regex.exec(input)) !== null) { | |
| if (match[1]) { | |
| matches.push(match[1]); | |
| } | |
| } | |
| return matches; | |
| } | |
| export function extractAllKeyNamesFromTemplateVars(input: string): string[] { | |
| const regex = /\{\{KEY\((.*?)\)\}\}/g; | |
| const matches = []; | |
| let match; | |
| while ((match = regex.exec(input)) !== null) { | |
| if (match[1]) { | |
| // Validate key name contains only alphanumeric characters and underscores, and doesn’t start with a digit | |
| if (!/^[a-zA-Z_][a-zA-Z0-9_]*$/.test(match[1])) { | |
| throw new Error( | |
| `Invalid vault key name: "${match[1]}". Key names must be valid environment variable names.` | |
| ); | |
| } | |
| matches.push(match[1]); | |
| } | |
| } | |
| return matches; | |
| } |
🤖 Prompt for AI Agents
In packages/core/src/helpers/AWSLambdaCode.helper.ts around lines 553 to 563,
the extracted vault key names from the template are not validated; update the
function to validate each extracted name against a standard environment-variable
pattern (/^[A-Za-z_][A-Za-z0-9_]*$/), collect any invalid names and either
filter them out or (preferably) throw an Error listing invalid keys so callers
know which entries are bad; ensure the function returns only the valid key names
when successful and include a clear error message naming the offending keys if
validation fails.
| async function fetchVaultSecret(keyName: string, agentTeamId: string): Promise<{ value: string, key: string }> { | ||
| const vaultSecret = await VaultHelper.getAgentKey(keyName, agentTeamId); | ||
| return { | ||
| value: vaultSecret, | ||
| key: keyName, | ||
| }; | ||
|
|
||
| } |
There was a problem hiding this comment.
Add error handling for missing vault keys
The fetchVaultSecret function doesn't handle cases where a vault key might not exist. This could cause the entire deployment to fail if a referenced key is missing.
Consider adding error handling:
async function fetchVaultSecret(keyName: string, agentTeamId: string): Promise<{ value: string, key: string }> {
- const vaultSecret = await VaultHelper.getAgentKey(keyName, agentTeamId);
- return {
- value: vaultSecret,
- key: keyName,
- };
+ try {
+ const vaultSecret = await VaultHelper.getAgentKey(keyName, agentTeamId);
+ return {
+ value: vaultSecret,
+ key: keyName,
+ };
+ } catch (error) {
+ console.error(`Failed to fetch vault key "${keyName}":`, error);
+ throw new Error(`Vault key "${keyName}" not found or inaccessible`);
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async function fetchVaultSecret(keyName: string, agentTeamId: string): Promise<{ value: string, key: string }> { | |
| const vaultSecret = await VaultHelper.getAgentKey(keyName, agentTeamId); | |
| return { | |
| value: vaultSecret, | |
| key: keyName, | |
| }; | |
| } | |
| async function fetchVaultSecret(keyName: string, agentTeamId: string): Promise<{ value: string, key: string }> { | |
| try { | |
| const vaultSecret = await VaultHelper.getAgentKey(keyName, agentTeamId); | |
| return { | |
| value: vaultSecret, | |
| key: keyName, | |
| }; | |
| } catch (error) { | |
| console.error(`Failed to fetch vault key "${keyName}":`, error); | |
| throw new Error(`Vault key "${keyName}" not found or inaccessible`); | |
| } | |
| } |
🤖 Prompt for AI Agents
In packages/core/src/helpers/AWSLambdaCode.helper.ts around lines 566 to 573,
fetchVaultSecret currently assumes VaultHelper.getAgentKey always returns a
value; add handling for missing or empty secrets by checking the returned
vaultSecret (null/undefined/empty string) and, if absent, either throw a
descriptive Error (e.g. `throw new Error(\`Vault secret not found for key
${keyName} and team ${agentTeamId}\`)`) or return a clearly documented fallback
structure indicating the missing key; ensure the caller contract is preserved
(update callers if you choose to throw) and include a short
processLogger.warn/error entry before throwing/returning to aid debugging.
| const [isLambdaExists, exisitingCodeHash, currentEnvVariables] = await Promise.all([ | ||
| getDeployedFunction(functionName, this.awsConfigs), | ||
| getDeployedCodeHash(agentId, codeUID), | ||
| getCurrentEnvironmentVariables(acRequest.candidate.id, input.code), | ||
| ]); |
There was a problem hiding this comment.
Incorrect parameter passed to getCurrentEnvironmentVariables
The function getCurrentEnvironmentVariables expects agentTeamId as the first parameter, but you're passing acRequest.candidate.id which is the agentId. This could lead to incorrect vault secret retrieval.
Consider retrieving the team ID first:
- getCurrentEnvironmentVariables(acRequest.candidate.id, input.code),
+ getCurrentEnvironmentVariables(/* need to get teamId from acRequest */, input.code),You may need to fetch the team ID using the AccountConnector service similar to how it's done in the helper functions.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
packages/core/src/subsystems/ComputeManager/Code.service/connectors/AWSLambdaCode.class.ts
around lines 34 to 38, getCurrentEnvironmentVariables is being called with
acRequest.candidate.id (agentId) but it expects agentTeamId; fetch the agent's
team ID first (use the AccountConnector or the same helper used elsewhere to
resolve team ID from the agent/account) and then call
getCurrentEnvironmentVariables(agentTeamId, input.code) so the correct vault
secrets are retrieved.
| fs.rmSync(`${directory}`, { recursive: true, force: true }); | ||
| fs.unlinkSync(`${directory}.zip`); | ||
| } catch (error) {} | ||
| } catch (error) { } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error logging in catch block
The empty catch block silently swallows errors during cleanup. Consider logging the error for debugging purposes.
- } catch (error) { }
+ } catch (error) {
+ console.debug('Cleanup error:', error);
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } catch (error) { } | |
| } catch (error) { | |
| console.debug('Cleanup error:', error); | |
| } |
🤖 Prompt for AI Agents
In
packages/core/src/subsystems/ComputeManager/Code.service/connectors/AWSLambdaCode.class.ts
around line 88, the catch block is empty and silently swallows errors during
cleanup; update the catch to log the error (including message and stack) with
the class/logger used in this file (e.g., this.logger?.error('Failed during
cleanup in AWSLambdaCode', error) or fall back to console.error if no logger),
ensure the caught value is handled as unknown and included in the log (e.g.,
String(error) and (error as Error).stack) to provide clear context for
debugging, and do not suppress the original error details.
2 participants
📝 Description
Added Support for Vault Key Usage inside NodeJS Code Component. Key can be used by using this string template:
{{KEY(keyNameInVault)}} where keyNameInVault is the actual key name in SmythOS Vault.
For using own keys, users need to add "lambda:UpdateFunctionConfiguration" permission in policy document for specified user.
🔗 Related Issues
🔧 Type of Change
✅ Checklist
Summary by CodeRabbit