Skip to content

Commit 847ce42

Browse files
waleedlatif1waleedlatif1
committed
fix(aws): address review round and close remaining empty-value leaks
Send no IAM usage filter for All, assign Secrets Manager create/update KMS key and tags explicitly, accept object or key/value IAM tag rows, reject non-array SES email tags, require DynamoDB parallel-scan segments together and below the total, restrict string scanIndexForward to true/false, and fix SES test typing.
1 parent d5d69ae commit 847ce42

7 files changed

Lines changed: 89 additions & 45 deletions

File tree

‎apps/sim/blocks/blocks/aws-stale-inputs.test.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { describe, expect, it } from 'vitest'
77
import { awsCloudformationCreateChangeSetContract } from '@/lib/api/contracts/tools/aws/cloudformation-create-change-set'
88
import { awsDynamodbScanContract } from '@/lib/api/contracts/tools/aws/dynamodb-scan'
99
import { awsIamCreateUserContract } from '@/lib/api/contracts/tools/aws/iam-create-user'
10+
import { awsIamListPoliciesContract } from '@/lib/api/contracts/tools/aws/iam-list-policies'
1011
import { awsSesSendTemplatedEmailContract } from '@/lib/api/contracts/tools/aws/ses-send-templated-email'
1112
import { CloudFormationBlock } from '@/blocks/blocks/cloudformation'
1213
import { DynamoDBBlock } from '@/blocks/blocks/dynamodb'
@@ -16,6 +17,7 @@ import type { BlockConfig } from '@/blocks/types'
1617
import { createChangeSetTool } from '@/tools/cloudformation/create_change_set'
1718
import { scanTool } from '@/tools/dynamodb/scan'
1819
import { createUserTool } from '@/tools/iam/create_user'
20+
import { listPoliciesTool } from '@/tools/iam/list_policies'
1921
import { sendTemplatedEmailTool } from '@/tools/ses/send_templated_email'
2022

2123
const AWS = { region: 'us-east-1', accessKeyId: 'AKIAEXAMPLE', secretAccessKey: 'secret' }
@@ -80,4 +82,15 @@ describe('AWS blocks keep stale subBlock values out of tool input', () => {
8082
const body = createUserTool.operation.input(params as never)
8183
expect(awsIamCreateUserContract.body?.safeParse(body).success).toBe(true)
8284
})
85+
86+
it('list policies with the All usage filter sends no filter', () => {
87+
const params = executorParams(IAMBlock, {
88+
...AWS,
89+
operation: 'list_policies',
90+
policyUsageFilter: 'all',
91+
})
92+
const body = listPoliciesTool.operation.input(params as never) as Record<string, unknown>
93+
expect(body.policyUsageFilter).toBeUndefined()
94+
expect(awsIamListPoliciesContract.body?.safeParse(body).success).toBe(true)
95+
})
8396
})

‎apps/sim/blocks/blocks/iam.ts‎

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,23 @@
1+
import { isRecordLike } from '@sim/utils/object'
12
import { IAMIcon } from '@/components/icons'
23
import type { BlockConfig, BlockMeta } from '@/blocks/types'
34
import { AuthMode, IntegrationType } from '@/blocks/types'
45
import { withSavedCredential } from '@/blocks/utils'
56

6-
/** Converts the Tags table (or a JSON object string) into the JSON object string the tools accept. */
7+
/**
8+
* Converts the Tags table, a `[{ key, value }]` list, a plain object, or a JSON object
9+
* string into the JSON object string the tools accept.
10+
*/
711
function serializeTags(tags: unknown): string | undefined {
812
if (typeof tags === 'string') return tags.trim() ? tags : undefined
13+
if (isRecordLike(tags)) return Object.keys(tags).length > 0 ? JSON.stringify(tags) : undefined
914
if (!Array.isArray(tags)) return undefined
1015
const obj: Record<string, string> = {}
1116
for (const row of tags) {
12-
const key = row?.cells?.key
13-
const value = row?.cells?.value
14-
if (key && value !== undefined) obj[key] = String(value)
17+
const entry = isRecordLike(row) && isRecordLike(row.cells) ? row.cells : row
18+
const key = isRecordLike(entry) ? entry.key : undefined
19+
const value = isRecordLike(entry) ? entry.value : undefined
20+
if (typeof key === 'string' && key && value !== undefined) obj[key] = String(value)
1521
}
1622
return Object.keys(obj).length > 0 ? JSON.stringify(obj) : undefined
1723
}
@@ -684,12 +690,11 @@ export const IAMBlock: BlockConfig = {
684690
case 'list_policies':
685691
if (rest.scope) result.scope = rest.scope
686692
if (onlyAttached === 'true' || onlyAttached === true) result.onlyAttached = true
687-
if (
693+
result.policyUsageFilter =
688694
rest.policyUsageFilter === 'PermissionsPolicy' ||
689695
rest.policyUsageFilter === 'PermissionsBoundary'
690-
) {
691-
result.policyUsageFilter = rest.policyUsageFilter
692-
}
696+
? rest.policyUsageFilter
697+
: undefined
693698
if (rest.pathPrefix) result.pathPrefix = rest.pathPrefix
694699
if (maxItems) {
695700
const parsed = Number.parseInt(String(maxItems), 10)

‎apps/sim/blocks/blocks/secrets_manager.ts‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -448,17 +448,18 @@ Return ONLY the JSON array - no explanations, no markdown code blocks.`,
448448
result.name = rest.name
449449
result.secretValue = rest.secretValue
450450
if (rest.description) result.description = rest.description
451-
if (rest.kmsKeyId) result.kmsKeyId = rest.kmsKeyId
452-
if (rest.createTags) {
453-
result.tags =
454-
typeof rest.createTags === 'string' ? JSON.parse(rest.createTags) : rest.createTags
455-
}
451+
result.kmsKeyId = rest.kmsKeyId || undefined
452+
result.tags = rest.createTags
453+
? typeof rest.createTags === 'string'
454+
? JSON.parse(rest.createTags)
455+
: rest.createTags
456+
: undefined
456457
break
457458
case 'update_secret':
458459
result.secretId = rest.secretId
459460
result.secretValue = rest.secretValue
460461
if (rest.description) result.description = rest.description
461-
if (rest.kmsKeyId) result.kmsKeyId = rest.kmsKeyId
462+
result.kmsKeyId = rest.kmsKeyId || undefined
462463
break
463464
case 'delete_secret':
464465
result.secretId = rest.secretId

‎apps/sim/blocks/blocks/ses.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -699,11 +699,16 @@ export const SESBlock: BlockConfig = {
699699
if (value === undefined || value === null || value === '') return undefined
700700
if (typeof value !== 'string') return value
701701
if (!value.trim()) return undefined
702+
let parsed: unknown
702703
try {
703-
return JSON.parse(value)
704+
parsed = JSON.parse(value)
704705
} catch {
705706
throw new Error('Email tags must be a valid JSON array of {"name", "value"} objects')
706707
}
708+
if (!Array.isArray(parsed)) {
709+
throw new Error('Email tags must be a valid JSON array of {"name", "value"} objects')
710+
}
711+
return parsed
707712
}
708713

709714
const sendAttachments = normalizeFileInput(attachments)

‎apps/sim/lib/api/contracts/tools/aws/dynamodb-query.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ const QuerySchema = z.object({
2525
limit: z.number().positive().optional(),
2626
exclusiveStartKey: z.record(z.string(), z.unknown()).optional(),
2727
scanIndexForward: z
28-
.union([z.boolean(), z.string()])
28+
.union([z.boolean(), z.enum(['true', 'false'])])
2929
.optional()
3030
.transform((val) => {
3131
if (val === false || val === 'false') return false

‎apps/sim/lib/api/contracts/tools/aws/dynamodb-scan.ts‎

Lines changed: 47 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -7,33 +7,53 @@ import type {
77
import { defineRouteContract } from '@/lib/api/contracts/types'
88
import { validateAwsRegion } from '@/lib/core/security/input-validation'
99

10-
const ScanSchema = z.object({
11-
region: z
12-
.string()
13-
.min(1, 'AWS region is required')
14-
.refine((v) => validateAwsRegion(v).isValid, {
15-
message: 'Invalid AWS region format (e.g., us-east-1, eu-west-2)',
16-
}),
17-
accessKeyId: z.string().min(1, 'AWS access key ID is required'),
18-
secretAccessKey: z.string().min(1, 'AWS secret access key is required'),
19-
tableName: z.string().min(1, 'Table name is required'),
20-
filterExpression: z.string().optional(),
21-
projectionExpression: z.string().optional(),
22-
expressionAttributeNames: z.record(z.string(), z.string()).optional(),
23-
expressionAttributeValues: z.record(z.string(), z.unknown()).optional(),
24-
limit: z.number().positive().optional(),
25-
exclusiveStartKey: z.record(z.string(), z.unknown()).optional(),
26-
indexName: z.string().optional(),
27-
consistentRead: z
28-
.union([z.boolean(), z.string()])
29-
.optional()
30-
.transform((val) => {
31-
if (val === true || val === 'true') return true
32-
return undefined
33-
}),
34-
segment: z.number().int().min(0).max(999999).optional(),
35-
totalSegments: z.number().int().min(1).max(1000000).optional(),
36-
})
10+
const ScanSchema = z
11+
.object({
12+
region: z
13+
.string()
14+
.min(1, 'AWS region is required')
15+
.refine((v) => validateAwsRegion(v).isValid, {
16+
message: 'Invalid AWS region format (e.g., us-east-1, eu-west-2)',
17+
}),
18+
accessKeyId: z.string().min(1, 'AWS access key ID is required'),
19+
secretAccessKey: z.string().min(1, 'AWS secret access key is required'),
20+
tableName: z.string().min(1, 'Table name is required'),
21+
filterExpression: z.string().optional(),
22+
projectionExpression: z.string().optional(),
23+
expressionAttributeNames: z.record(z.string(), z.string()).optional(),
24+
expressionAttributeValues: z.record(z.string(), z.unknown()).optional(),
25+
limit: z.number().positive().optional(),
26+
exclusiveStartKey: z.record(z.string(), z.unknown()).optional(),
27+
indexName: z.string().optional(),
28+
consistentRead: z
29+
.union([z.boolean(), z.string()])
30+
.optional()
31+
.transform((val) => {
32+
if (val === true || val === 'true') return true
33+
return undefined
34+
}),
35+
segment: z.number().int().min(0).max(999999).optional(),
36+
totalSegments: z.number().int().min(1).max(1000000).optional(),
37+
})
38+
.superRefine((value, ctx) => {
39+
if ((value.segment === undefined) !== (value.totalSegments === undefined)) {
40+
ctx.addIssue({
41+
code: 'custom',
42+
path: ['segment'],
43+
message: 'Segment and Total Segments must be set together for a parallel scan',
44+
})
45+
} else if (
46+
value.segment !== undefined &&
47+
value.totalSegments !== undefined &&
48+
value.segment >= value.totalSegments
49+
) {
50+
ctx.addIssue({
51+
code: 'custom',
52+
path: ['segment'],
53+
message: 'Segment must be less than Total Segments',
54+
})
55+
}
56+
})
3757

3858
const ScanResponseSchema = z.object({
3959
message: z.string(),

‎apps/sim/lib/internal/ses/operations.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,8 +111,8 @@ describe('SES send operations', () => {
111111
bodyText: 'Hello',
112112
})
113113

114-
const [display, address] = sentInput().FromEmailAddress.split(' <')
115-
const words = display.split(' ')
114+
const [display, address] = String(sentInput().FromEmailAddress).split(' <')
115+
const words: string[] = display.split(' ')
116116
expect(words.length).toBeGreaterThan(1)
117117
for (const word of words) expect(word.length).toBeLessThanOrEqual(75)
118118
const decoded = words

0 commit comments

Comments
 (0)