diff --git a/lat.md/lat.md b/lat.md/lat.md new file mode 100644 index 0000000..8eb0df5 --- /dev/null +++ b/lat.md/lat.md @@ -0,0 +1,3 @@ +This directory defines the high-level concepts, business logic, and architecture of this project using markdown. It is managed by [lat.md](https://www.npmjs.com/package/lat.md) — a tool that anchors source code to these definitions. Install the `lat` command with `npm i -g lat.md` and run `lat --help`. + +- [[security]] — Security invariants: parameterized SQL, admin authorization on the feedback endpoint, device serial number scoping. diff --git a/lat.md/security.md b/lat.md/security.md new file mode 100644 index 0000000..c91752e --- /dev/null +++ b/lat.md/security.md @@ -0,0 +1,45 @@ +# Security and Data Access + +Security invariants for the Songbird backend Lambda layer — parameterized SQL, authorization checks, and device-scoped data access. + +## Parameterized SQL Queries + +All Aurora queries use RDS Data API `parameters` — never string interpolation or manual quote-escaping. This applies to every `ExecuteStatementCommand` call across the analytics layer. + +The two Lambda functions that write to `analytics.rag_documents` must follow this rule: + +- [[songbird-infrastructure/lambda/shared/rag-retrieval.ts#retrieveRelevantContext]] — vector similarity search passes the embedding and title exclusion list as named parameters (`:embedding`, `:limit`, `:p0`…`:pN`). +- [[songbird-infrastructure/lambda/analytics/feedback.ts#indexPositiveFeedback]] — DELETE and INSERT for the upsert pattern use `:title`, `:content`, `:embedding`, `:metadata` parameters. + +String values that were previously escaped with `.replace(/'/g, "''")` must not return. Parameterized queries eliminate that class of bug entirely. + +## Admin Authorization on Feedback Endpoint + +The `GET /analytics/feedback` route returns all users' query history (questions, generated SQL, usernames). It must verify the caller belongs to the Cognito `Admin` group before returning any data. + +The check reads `cognito:groups` from the JWT claims injected by API Gateway's JWT authorizer. A missing or non-Admin group claim returns 403. This is a defense-in-depth check — the endpoint may also be restricted at the API Gateway level, but the Lambda must not rely solely on that. + +See [[songbird-infrastructure/lambda/analytics/feedback.ts#handler]]. + +## Device Serial Number Authorization + +Chat query requests must supply an explicit `deviceSerialNumbers` array. If the array is absent or empty, the handler returns 403 immediately. + +The previous behavior — falling back to `SELECT DISTINCT serial_number FROM analytics.devices` — granted unrestricted data access to any caller who omitted the field. That fallback has been removed. + +See [[songbird-infrastructure/lambda/analytics/chat-query.ts#handler]]. + +## Integer Query Parameter Safety + +Query-string parameters that feed into DynamoDB `Limit` values must be parsed with [[songbird-infrastructure/lambda/shared/utils.ts#parseIntParam]], not bare `parseInt()`. Bare `parseInt()` returns `NaN` for non-numeric input, which propagates into DynamoDB and causes 500 errors with no useful message. + +`parseIntParam` returns the default when the value is missing, non-numeric, or less than 1, and optionally clamps the result to a maximum. All five GET-list handlers (`api-devices`, `api-alerts`, `api-telemetry`, `api-activity`, `api-journeys`) use this helper. + +## JSON Body Error Handling in Mutation Paths + +POST/PATCH/DELETE handlers that call `JSON.parse(event.body)` must wrap the call in a try/catch that returns a 400 with a descriptive error. Uncaught `SyntaxError` from malformed JSON propagates to the outer catch and becomes an opaque 500. + +Handlers that apply this guard: +- [[songbird-infrastructure/lambda/api-devices/index.ts#mergeDevices]] — `POST /devices/merge` +- [[songbird-infrastructure/lambda/api-devices/index.ts#updateDeviceBySerial]] — `PATCH /devices/{serial_number}` +- [[songbird-infrastructure/lambda/api-commands/index.ts#sendCommand]] — `POST /devices/{serial_number}/commands` diff --git a/songbird-firmware/src/audio/SongbirdAudio.cpp b/songbird-firmware/src/audio/SongbirdAudio.cpp index 2f519f6..d283931 100644 --- a/songbird-firmware/src/audio/SongbirdAudio.cpp +++ b/songbird-firmware/src/audio/SongbirdAudio.cpp @@ -196,7 +196,11 @@ void audioPlayMelody(const Melody* melody, uint8_t volume) { // Small gap between notes (unless it's a rest) if (melody->notes[i] != NOTE_REST && i < melody->length - 1) { - vTaskDelay(pdMS_TO_TICKS(TONE_GAP_MS)); + if (useRtosPrimitives()) { + vTaskDelay(pdMS_TO_TICKS(TONE_GAP_MS)); + } else { + delay(TONE_GAP_MS); // Arduino delay — safe before scheduler starts + } } } } diff --git a/songbird-firmware/src/commands/SongbirdEnv.cpp b/songbird-firmware/src/commands/SongbirdEnv.cpp index 6243818..26fb080 100644 --- a/songbird-firmware/src/commands/SongbirdEnv.cpp +++ b/songbird-firmware/src/commands/SongbirdEnv.cpp @@ -495,163 +495,168 @@ void envLogConfigChanges(const SongbirdConfig* oldConfig, const SongbirdConfig* return; } - Serial.println("[Env] Configuration changed from Notehub:"); + #ifdef DEBUG_MODE + DEBUG_SERIAL.println("[Env] Configuration changed from Notehub:"); // Mode if (oldConfig->mode != newConfig->mode) { - Serial.print(" mode: "); - Serial.print(envGetModeName(oldConfig->mode)); - Serial.print(" -> "); - Serial.println(envGetModeName(newConfig->mode)); + DEBUG_SERIAL.print(" mode: "); + DEBUG_SERIAL.print(envGetModeName(oldConfig->mode)); + DEBUG_SERIAL.print(" -> "); + DEBUG_SERIAL.println(envGetModeName(newConfig->mode)); } // Timing if (oldConfig->gpsIntervalMin != newConfig->gpsIntervalMin) { - Serial.print(" gps_interval_min: "); - Serial.print(oldConfig->gpsIntervalMin); - Serial.print(" -> "); - Serial.println(newConfig->gpsIntervalMin); + DEBUG_SERIAL.print(" gps_interval_min: "); + DEBUG_SERIAL.print(oldConfig->gpsIntervalMin); + DEBUG_SERIAL.print(" -> "); + DEBUG_SERIAL.println(newConfig->gpsIntervalMin); } if (oldConfig->syncIntervalMin != newConfig->syncIntervalMin) { - Serial.print(" sync_interval_min: "); - Serial.print(oldConfig->syncIntervalMin); - Serial.print(" -> "); - Serial.println(newConfig->syncIntervalMin); + DEBUG_SERIAL.print(" sync_interval_min: "); + DEBUG_SERIAL.print(oldConfig->syncIntervalMin); + DEBUG_SERIAL.print(" -> "); + DEBUG_SERIAL.println(newConfig->syncIntervalMin); } if (oldConfig->heartbeatHours != newConfig->heartbeatHours) { - Serial.print(" heartbeat_hours: "); - Serial.print(oldConfig->heartbeatHours); - Serial.print(" -> "); - Serial.println(newConfig->heartbeatHours); + DEBUG_SERIAL.print(" heartbeat_hours: "); + DEBUG_SERIAL.print(oldConfig->heartbeatHours); + DEBUG_SERIAL.print(" -> "); + DEBUG_SERIAL.println(newConfig->heartbeatHours); } // Temperature alerts if (oldConfig->tempAlertHighC != newConfig->tempAlertHighC) { - Serial.print(" temp_alert_high_c: "); - Serial.print(oldConfig->tempAlertHighC); - Serial.print(" -> "); - Serial.println(newConfig->tempAlertHighC); + DEBUG_SERIAL.print(" temp_alert_high_c: "); + DEBUG_SERIAL.print(oldConfig->tempAlertHighC); + DEBUG_SERIAL.print(" -> "); + DEBUG_SERIAL.println(newConfig->tempAlertHighC); } if (oldConfig->tempAlertLowC != newConfig->tempAlertLowC) { - Serial.print(" temp_alert_low_c: "); - Serial.print(oldConfig->tempAlertLowC); - Serial.print(" -> "); - Serial.println(newConfig->tempAlertLowC); + DEBUG_SERIAL.print(" temp_alert_low_c: "); + DEBUG_SERIAL.print(oldConfig->tempAlertLowC); + DEBUG_SERIAL.print(" -> "); + DEBUG_SERIAL.println(newConfig->tempAlertLowC); } // Humidity alerts if (oldConfig->humidityAlertHigh != newConfig->humidityAlertHigh) { - Serial.print(" humidity_alert_high: "); - Serial.print(oldConfig->humidityAlertHigh); - Serial.print(" -> "); - Serial.println(newConfig->humidityAlertHigh); + DEBUG_SERIAL.print(" humidity_alert_high: "); + DEBUG_SERIAL.print(oldConfig->humidityAlertHigh); + DEBUG_SERIAL.print(" -> "); + DEBUG_SERIAL.println(newConfig->humidityAlertHigh); } if (oldConfig->humidityAlertLow != newConfig->humidityAlertLow) { - Serial.print(" humidity_alert_low: "); - Serial.print(oldConfig->humidityAlertLow); - Serial.print(" -> "); - Serial.println(newConfig->humidityAlertLow); + DEBUG_SERIAL.print(" humidity_alert_low: "); + DEBUG_SERIAL.print(oldConfig->humidityAlertLow); + DEBUG_SERIAL.print(" -> "); + DEBUG_SERIAL.println(newConfig->humidityAlertLow); } // Pressure and voltage alerts if (oldConfig->pressureAlertDelta != newConfig->pressureAlertDelta) { - Serial.print(" pressure_alert_delta: "); - Serial.print(oldConfig->pressureAlertDelta); - Serial.print(" -> "); - Serial.println(newConfig->pressureAlertDelta); + DEBUG_SERIAL.print(" pressure_alert_delta: "); + DEBUG_SERIAL.print(oldConfig->pressureAlertDelta); + DEBUG_SERIAL.print(" -> "); + DEBUG_SERIAL.println(newConfig->pressureAlertDelta); } if (oldConfig->voltageAlertLow != newConfig->voltageAlertLow) { - Serial.print(" voltage_alert_low: "); - Serial.print(oldConfig->voltageAlertLow); - Serial.print(" -> "); - Serial.println(newConfig->voltageAlertLow); + DEBUG_SERIAL.print(" voltage_alert_low: "); + DEBUG_SERIAL.print(oldConfig->voltageAlertLow); + DEBUG_SERIAL.print(" -> "); + DEBUG_SERIAL.println(newConfig->voltageAlertLow); } // Motion if (oldConfig->motionSensitivity != newConfig->motionSensitivity) { - Serial.print(" motion_sensitivity: "); - Serial.print(getSensitivityName(oldConfig->motionSensitivity)); - Serial.print(" -> "); - Serial.println(getSensitivityName(newConfig->motionSensitivity)); + DEBUG_SERIAL.print(" motion_sensitivity: "); + DEBUG_SERIAL.print(getSensitivityName(oldConfig->motionSensitivity)); + DEBUG_SERIAL.print(" -> "); + DEBUG_SERIAL.println(getSensitivityName(newConfig->motionSensitivity)); } if (oldConfig->motionWakeEnabled != newConfig->motionWakeEnabled) { - Serial.print(" motion_wake_enabled: "); - Serial.print(oldConfig->motionWakeEnabled ? "true" : "false"); - Serial.print(" -> "); - Serial.println(newConfig->motionWakeEnabled ? "true" : "false"); + DEBUG_SERIAL.print(" motion_wake_enabled: "); + DEBUG_SERIAL.print(oldConfig->motionWakeEnabled ? "true" : "false"); + DEBUG_SERIAL.print(" -> "); + DEBUG_SERIAL.println(newConfig->motionWakeEnabled ? "true" : "false"); } // Audio if (oldConfig->audioEnabled != newConfig->audioEnabled) { - Serial.print(" audio_enabled: "); - Serial.print(oldConfig->audioEnabled ? "true" : "false"); - Serial.print(" -> "); - Serial.println(newConfig->audioEnabled ? "true" : "false"); + DEBUG_SERIAL.print(" audio_enabled: "); + DEBUG_SERIAL.print(oldConfig->audioEnabled ? "true" : "false"); + DEBUG_SERIAL.print(" -> "); + DEBUG_SERIAL.println(newConfig->audioEnabled ? "true" : "false"); } if (oldConfig->audioVolume != newConfig->audioVolume) { - Serial.print(" audio_volume: "); - Serial.print(oldConfig->audioVolume); - Serial.print(" -> "); - Serial.println(newConfig->audioVolume); + DEBUG_SERIAL.print(" audio_volume: "); + DEBUG_SERIAL.print(oldConfig->audioVolume); + DEBUG_SERIAL.print(" -> "); + DEBUG_SERIAL.println(newConfig->audioVolume); } if (oldConfig->audioAlertsOnly != newConfig->audioAlertsOnly) { - Serial.print(" audio_alerts_only: "); - Serial.print(oldConfig->audioAlertsOnly ? "true" : "false"); - Serial.print(" -> "); - Serial.println(newConfig->audioAlertsOnly ? "true" : "false"); + DEBUG_SERIAL.print(" audio_alerts_only: "); + DEBUG_SERIAL.print(oldConfig->audioAlertsOnly ? "true" : "false"); + DEBUG_SERIAL.print(" -> "); + DEBUG_SERIAL.println(newConfig->audioAlertsOnly ? "true" : "false"); } // Commands if (oldConfig->cmdWakeEnabled != newConfig->cmdWakeEnabled) { - Serial.print(" cmd_wake_enabled: "); - Serial.print(oldConfig->cmdWakeEnabled ? "true" : "false"); - Serial.print(" -> "); - Serial.println(newConfig->cmdWakeEnabled ? "true" : "false"); + DEBUG_SERIAL.print(" cmd_wake_enabled: "); + DEBUG_SERIAL.print(oldConfig->cmdWakeEnabled ? "true" : "false"); + DEBUG_SERIAL.print(" -> "); + DEBUG_SERIAL.println(newConfig->cmdWakeEnabled ? "true" : "false"); } if (oldConfig->cmdAckEnabled != newConfig->cmdAckEnabled) { - Serial.print(" cmd_ack_enabled: "); - Serial.print(oldConfig->cmdAckEnabled ? "true" : "false"); - Serial.print(" -> "); - Serial.println(newConfig->cmdAckEnabled ? "true" : "false"); + DEBUG_SERIAL.print(" cmd_ack_enabled: "); + DEBUG_SERIAL.print(oldConfig->cmdAckEnabled ? "true" : "false"); + DEBUG_SERIAL.print(" -> "); + DEBUG_SERIAL.println(newConfig->cmdAckEnabled ? "true" : "false"); } if (oldConfig->locateDurationSec != newConfig->locateDurationSec) { - Serial.print(" locate_duration_sec: "); - Serial.print(oldConfig->locateDurationSec); - Serial.print(" -> "); - Serial.println(newConfig->locateDurationSec); + DEBUG_SERIAL.print(" locate_duration_sec: "); + DEBUG_SERIAL.print(oldConfig->locateDurationSec); + DEBUG_SERIAL.print(" -> "); + DEBUG_SERIAL.println(newConfig->locateDurationSec); } // Misc if (oldConfig->ledEnabled != newConfig->ledEnabled) { - Serial.print(" led_enabled: "); - Serial.print(oldConfig->ledEnabled ? "true" : "false"); - Serial.print(" -> "); - Serial.println(newConfig->ledEnabled ? "true" : "false"); + DEBUG_SERIAL.print(" led_enabled: "); + DEBUG_SERIAL.print(oldConfig->ledEnabled ? "true" : "false"); + DEBUG_SERIAL.print(" -> "); + DEBUG_SERIAL.println(newConfig->ledEnabled ? "true" : "false"); } if (oldConfig->debugMode != newConfig->debugMode) { - Serial.print(" debug_mode: "); - Serial.print(oldConfig->debugMode ? "true" : "false"); - Serial.print(" -> "); - Serial.println(newConfig->debugMode ? "true" : "false"); + DEBUG_SERIAL.print(" debug_mode: "); + DEBUG_SERIAL.print(oldConfig->debugMode ? "true" : "false"); + DEBUG_SERIAL.print(" -> "); + DEBUG_SERIAL.println(newConfig->debugMode ? "true" : "false"); } // GPS Power Management if (oldConfig->gpsPowerSaveEnabled != newConfig->gpsPowerSaveEnabled) { - Serial.print(" gps_power_save_enabled: "); - Serial.print(oldConfig->gpsPowerSaveEnabled ? "true" : "false"); - Serial.print(" -> "); - Serial.println(newConfig->gpsPowerSaveEnabled ? "true" : "false"); + DEBUG_SERIAL.print(" gps_power_save_enabled: "); + DEBUG_SERIAL.print(oldConfig->gpsPowerSaveEnabled ? "true" : "false"); + DEBUG_SERIAL.print(" -> "); + DEBUG_SERIAL.println(newConfig->gpsPowerSaveEnabled ? "true" : "false"); } if (oldConfig->gpsSignalTimeoutMin != newConfig->gpsSignalTimeoutMin) { - Serial.print(" gps_signal_timeout_min: "); - Serial.print(oldConfig->gpsSignalTimeoutMin); - Serial.print(" -> "); - Serial.println(newConfig->gpsSignalTimeoutMin); + DEBUG_SERIAL.print(" gps_signal_timeout_min: "); + DEBUG_SERIAL.print(oldConfig->gpsSignalTimeoutMin); + DEBUG_SERIAL.print(" -> "); + DEBUG_SERIAL.println(newConfig->gpsSignalTimeoutMin); } if (oldConfig->gpsRetryIntervalMin != newConfig->gpsRetryIntervalMin) { - Serial.print(" gps_retry_interval_min: "); - Serial.print(oldConfig->gpsRetryIntervalMin); - Serial.print(" -> "); - Serial.println(newConfig->gpsRetryIntervalMin); + DEBUG_SERIAL.print(" gps_retry_interval_min: "); + DEBUG_SERIAL.print(oldConfig->gpsRetryIntervalMin); + DEBUG_SERIAL.print(" -> "); + DEBUG_SERIAL.println(newConfig->gpsRetryIntervalMin); } + #else + (void)oldConfig; + (void)newConfig; + #endif } diff --git a/songbird-infrastructure/lambda/analytics/chat-query.ts b/songbird-infrastructure/lambda/analytics/chat-query.ts index 2f5552b..6644884 100644 --- a/songbird-infrastructure/lambda/analytics/chat-query.ts +++ b/songbird-infrastructure/lambda/analytics/chat-query.ts @@ -464,35 +464,21 @@ export const handler = async (event: APIGatewayProxyEvent): Promise record[0]?.stringValue) - .filter((sn): sn is string => !!sn); - - // Fallback: also check telemetry table if devices table is empty - if (deviceSerialNumbers.length === 0) { - const telemetryResult = await rds.send(new ExecuteStatementCommand({ - resourceArn: CLUSTER_ARN, - secretArn: SECRET_ARN, - database: DATABASE_NAME, - sql: 'SELECT DISTINCT serial_number FROM analytics.telemetry LIMIT 100', - })); - - deviceSerialNumbers = (telemetryResult.records || []) - .map(record => record[0]?.stringValue) - .filter((sn): sn is string => !!sn); - } + if (deviceSerialNumbers.length === 0) { + return { + statusCode: 403, + headers: { + 'Content-Type': 'application/json', + 'Access-Control-Allow-Origin': '*', + }, + body: JSON.stringify({ error: 'deviceSerialNumbers is required' }), + }; } // Look up the user's assigned device from DynamoDB devices table diff --git a/songbird-infrastructure/lambda/analytics/feedback.test.ts b/songbird-infrastructure/lambda/analytics/feedback.test.ts index d7109c5..b83f211 100644 --- a/songbird-infrastructure/lambda/analytics/feedback.test.ts +++ b/songbird-infrastructure/lambda/analytics/feedback.test.ts @@ -69,7 +69,10 @@ describe('analytics/feedback handler', () => { const result = await handler(makeEvent({ httpMethod: 'GET', - requestContext: { http: { method: 'GET', path: '/analytics/feedback' } }, + requestContext: { + http: { method: 'GET', path: '/analytics/feedback' }, + authorizer: { jwt: { claims: { 'cognito:groups': 'Admin' } } }, + }, })); expect(result.statusCode).toBe(200); @@ -86,7 +89,10 @@ describe('analytics/feedback handler', () => { const result = await handler(makeEvent({ httpMethod: 'GET', - requestContext: { http: { method: 'GET', path: '/analytics/feedback' } }, + requestContext: { + http: { method: 'GET', path: '/analytics/feedback' }, + authorizer: { jwt: { claims: { 'cognito:groups': 'Admin' } } }, + }, })); expect(result.statusCode).toBe(200); @@ -107,7 +113,10 @@ describe('analytics/feedback handler', () => { const result = await handler(makeEvent({ httpMethod: 'GET', - requestContext: { http: { method: 'GET', path: '/analytics/feedback' } }, + requestContext: { + http: { method: 'GET', path: '/analytics/feedback' }, + authorizer: { jwt: { claims: { 'cognito:groups': 'Admin' } } }, + }, queryStringParameters: { limit: '2' }, })); @@ -116,6 +125,18 @@ describe('analytics/feedback handler', () => { expect(body.items.length).toBe(2); }); + it('GET returns 403 when caller is not in the Admin group', async () => { + const result = await handler(makeEvent({ + httpMethod: 'GET', + requestContext: { + http: { method: 'GET', path: '/analytics/feedback' }, + authorizer: { jwt: { claims: { 'cognito:groups': 'Viewer' } } }, + }, + })); + + expect(result.statusCode).toBe(403); + }); + // ─── DELETE ────────────────────────────────────────────────────────── it('DELETE removes feedback record by userEmail and ratedAt', async () => { diff --git a/songbird-infrastructure/lambda/analytics/feedback.ts b/songbird-infrastructure/lambda/analytics/feedback.ts index 903dc71..5d641fe 100644 --- a/songbird-infrastructure/lambda/analytics/feedback.ts +++ b/songbird-infrastructure/lambda/analytics/feedback.ts @@ -118,33 +118,38 @@ async function indexPositiveFeedback(req: FeedbackRequest): Promise { const embedding = await embedText(content); const embeddingStr = `[${embedding.join(',')}]`; - const titleEscaped = title.replace(/'/g, "''"); - const contentEscaped = content.replace(/'/g, "''"); - const metadataEscaped = metadata.replace(/'/g, "''"); - - // Delete existing by title then insert (upsert pattern) - await rds.send(new ExecuteStatementCommand({ + const connectionParams = { resourceArn: CLUSTER_ARN, secretArn: SECRET_ARN, database: DATABASE_NAME, - sql: `DELETE FROM analytics.rag_documents WHERE title = '${titleEscaped}'`, + }; + + // Delete existing by title then insert (upsert pattern) + await rds.send(new ExecuteStatementCommand({ + ...connectionParams, + sql: `DELETE FROM analytics.rag_documents WHERE title = :title`, + parameters: [{ name: 'title', value: { stringValue: title } }], })); await rds.send(new ExecuteStatementCommand({ - resourceArn: CLUSTER_ARN, - secretArn: SECRET_ARN, - database: DATABASE_NAME, + ...connectionParams, sql: ` INSERT INTO analytics.rag_documents (doc_type, title, content, embedding, metadata, pinned) VALUES ( 'example', - '${titleEscaped}', - '${contentEscaped}', - '${embeddingStr}'::vector, - '${metadataEscaped}'::jsonb, + :title, + :content, + :embedding::vector, + :metadata::jsonb, FALSE ) `, + parameters: [ + { name: 'title', value: { stringValue: title } }, + { name: 'content', value: { stringValue: content } }, + { name: 'embedding', value: { stringValue: embeddingStr } }, + { name: 'metadata', value: { stringValue: metadata } }, + ], })); console.log(`Indexed positive feedback example: "${title}"`); @@ -186,6 +191,15 @@ export const handler = async (event: APIGatewayProxyEvent): Promise { - const result = await docClient.send(new GetCommand({ - TableName: DEVICE_ALIASES_TABLE, - Key: { serial_number: serialNumber }, - })); - - if (!result.Item) { - return []; - } - - // Return current device_uid plus any historical ones - const deviceUids = [result.Item.device_uid]; - if (result.Item.previous_device_uids && Array.isArray(result.Item.previous_device_uids)) { - deviceUids.push(...result.Item.previous_device_uids); - } - return deviceUids; -} export const handler = async (event: APIGatewayProxyEvent): Promise => { const method = (event.requestContext as any)?.http?.method || event.httpMethod; @@ -107,7 +90,7 @@ async function listAlerts( const queryParams = event.queryStringParameters || {}; const serialNumber = queryParams.serial_number || queryParams.device_uid; // Support both for backwards compat const acknowledged = queryParams.acknowledged; - const limit = parseInt(queryParams.limit || '100'); + const limit = parseIntParam(queryParams.limit, 100, 500); let items: any[] = []; @@ -153,7 +136,7 @@ async function listAlerts( TableName: ALERTS_TABLE, IndexName: 'status-index', KeyConditionExpression: 'acknowledged = :ack', - ExpressionAttributeValues: { ':ack': 'false' }, + ExpressionAttributeValues: { ':ack': ACKNOWLEDGED.FALSE }, ScanIndexForward: false, Limit: limit, }); @@ -175,7 +158,7 @@ async function listAlerts( } // Calculate stats - const activeCount = items.filter(a => a.acknowledged === 'false').length; + const activeCount = items.filter(a => a.acknowledged === ACKNOWLEDGED.FALSE).length; return { statusCode: 200, @@ -237,7 +220,7 @@ async function acknowledgeAlert( Key: { alert_id: alertId }, UpdateExpression: 'SET acknowledged = :ack, acknowledged_at = :ack_at, acknowledged_by = :ack_by', ExpressionAttributeValues: { - ':ack': 'true', + ':ack': ACKNOWLEDGED.TRUE, ':ack_at': now, ':ack_by': acknowledgedBy, }, @@ -294,10 +277,10 @@ async function bulkAcknowledgeAlerts( UpdateExpression: 'SET acknowledged = :ack, acknowledged_at = :ack_at, acknowledged_by = :ack_by', ConditionExpression: 'acknowledged = :not_ack', ExpressionAttributeValues: { - ':ack': 'true', + ':ack': ACKNOWLEDGED.TRUE, ':ack_at': now, ':ack_by': acknowledgedBy, - ':not_ack': 'false', + ':not_ack': ACKNOWLEDGED.FALSE, }, ReturnValues: 'ALL_NEW', }); diff --git a/songbird-infrastructure/lambda/api-commands/index.ts b/songbird-infrastructure/lambda/api-commands/index.ts index 08eca75..33df71c 100644 --- a/songbird-infrastructure/lambda/api-commands/index.ts +++ b/songbird-infrastructure/lambda/api-commands/index.ts @@ -206,7 +206,16 @@ async function sendCommand( }; } - const request = JSON.parse(event.body); + let request: { cmd?: string; params?: unknown }; + try { + request = JSON.parse(event.body); + } catch { + return { + statusCode: 400, + headers, + body: JSON.stringify({ error: 'Invalid JSON in request body' }), + }; + } const { cmd, params } = request; // Validate command diff --git a/songbird-infrastructure/lambda/api-devices/index.ts b/songbird-infrastructure/lambda/api-devices/index.ts index 24a362c..484e5c7 100644 --- a/songbird-infrastructure/lambda/api-devices/index.ts +++ b/songbird-infrastructure/lambda/api-devices/index.ts @@ -19,9 +19,12 @@ import { } from '@aws-sdk/lib-dynamodb'; import { APIGatewayProxyEvent, APIGatewayProxyResult } from 'aws-lambda'; import { resolveDevice, getAliasBySerial } from '../shared/device-lookup'; +import { parseIntParam } from '../shared/utils'; const ddbClient = new DynamoDBClient({}); -const docClient = DynamoDBDocumentClient.from(ddbClient); +const docClient = DynamoDBDocumentClient.from(ddbClient, { + marshallOptions: { removeUndefinedValues: true }, +}); const DEVICES_TABLE = process.env.DEVICES_TABLE!; const DEVICE_ALIASES_TABLE = process.env.DEVICE_ALIASES_TABLE || 'songbird-device-aliases'; @@ -89,7 +92,7 @@ async function listDevices( const queryParams = event.queryStringParameters || {}; const fleet = queryParams.fleet; const status = queryParams.status; - const limit = parseInt(queryParams.limit || '100'); + const limit = parseIntParam(queryParams.limit, 100, 500); let items: any[] = []; @@ -215,7 +218,16 @@ async function updateDeviceBySerial( }; } - const updates = JSON.parse(body); + let updates: Record; + try { + updates = JSON.parse(body); + } catch { + return { + statusCode: 400, + headers, + body: JSON.stringify({ error: 'Invalid JSON in request body' }), + }; + } // Only allow certain fields to be updated (removed serial_number - it's now immutable) const allowedFields = ['name', 'assigned_to', 'assigned_to_name', 'fleet', 'notes']; @@ -421,7 +433,17 @@ async function mergeDevices( }; } - const { source_serial_number, target_serial_number } = JSON.parse(event.body); + let parsedBody: { source_serial_number?: string; target_serial_number?: string }; + try { + parsedBody = JSON.parse(event.body); + } catch { + return { + statusCode: 400, + headers, + body: JSON.stringify({ error: 'Invalid JSON in request body' }), + }; + } + const { source_serial_number, target_serial_number } = parsedBody; if (!source_serial_number || !target_serial_number) { return { diff --git a/songbird-infrastructure/lambda/api-ingest/index.ts b/songbird-infrastructure/lambda/api-ingest/index.ts index 8f53d71..b7d046f 100644 --- a/songbird-infrastructure/lambda/api-ingest/index.ts +++ b/songbird-infrastructure/lambda/api-ingest/index.ts @@ -5,11 +5,13 @@ * Processes incoming Songbird events and writes to DynamoDB. */ +import { randomUUID } from 'crypto'; import { DynamoDBClient } from '@aws-sdk/client-dynamodb'; import { DynamoDBDocumentClient, PutCommand, UpdateCommand, QueryCommand, GetCommand } from '@aws-sdk/lib-dynamodb'; import { SNSClient, PublishCommand } from '@aws-sdk/client-sns'; import { APIGatewayProxyEvent, APIGatewayProxyResult } from 'aws-lambda'; import { handleDeviceAlias } from '../shared/device-lookup'; +import { ACKNOWLEDGED } from '../shared/constants'; // Initialize clients const ddbClient = new DynamoDBClient({}); @@ -593,7 +595,7 @@ async function createLowBatteryAlert(event: SongbirdEvent): Promise { const now = Date.now(); const ttl = Math.floor(now / 1000) + TTL_SECONDS; - const alertId = `alert_${event.device_uid}_${now}_${Math.random().toString(36).substring(7)}`; + const alertId = `alert_${randomUUID()}`; const alertRecord = { alert_id: alertId, @@ -605,7 +607,7 @@ async function createLowBatteryAlert(event: SongbirdEvent): Promise { message: `Device restarted due to low battery (${event.body.voltage?.toFixed(2)}V)`, created_at: now, event_timestamp: event.timestamp * 1000, - acknowledged: 'false', + acknowledged: ACKNOWLEDGED.FALSE, ttl, location: event.location ? { lat: event.location.lat, @@ -700,7 +702,7 @@ async function createGpsPowerSaveAlert(event: SongbirdEvent): Promise { const now = Date.now(); const ttl = Math.floor(now / 1000) + TTL_SECONDS; - const alertId = `alert_${event.device_uid}_${now}_${Math.random().toString(36).substring(7)}`; + const alertId = `alert_${randomUUID()}`; const alertRecord = { alert_id: alertId, @@ -711,7 +713,7 @@ async function createGpsPowerSaveAlert(event: SongbirdEvent): Promise { message: 'GPS disabled for power saving - unable to acquire satellite signal', created_at: now, event_timestamp: event.timestamp * 1000, - acknowledged: 'false', + acknowledged: ACKNOWLEDGED.FALSE, ttl, location: event.location ? { lat: event.location.lat, @@ -803,7 +805,7 @@ async function createNoSatAlert(event: SongbirdEvent): Promise { const now = Date.now(); const ttl = Math.floor(now / 1000) + TTL_SECONDS; - const alertId = `alert_${event.device_uid}_${now}_${Math.random().toString(36).substring(7)}`; + const alertId = `alert_${randomUUID()}`; const alertRecord = { alert_id: alertId, @@ -814,7 +816,7 @@ async function createNoSatAlert(event: SongbirdEvent): Promise { message: 'Unable to obtain GPS location', created_at: now, event_timestamp: event.timestamp * 1000, - acknowledged: 'false', + acknowledged: ACKNOWLEDGED.FALSE, ttl, location: event.location ? { lat: event.location.lat, @@ -1121,7 +1123,7 @@ async function storeAlert(event: SongbirdEvent): Promise { const ttl = Math.floor(now / 1000) + TTL_SECONDS; // Generate a unique alert ID - const alertId = `alert_${event.device_uid}_${now}_${Math.random().toString(36).substring(7)}`; + const alertId = `alert_${randomUUID()}`; const alertRecord = { alert_id: alertId, @@ -1134,7 +1136,7 @@ async function storeAlert(event: SongbirdEvent): Promise { message: event.body.message || '', created_at: now, event_timestamp: event.timestamp * 1000, - acknowledged: 'false', // String for GSI partition key + acknowledged: ACKNOWLEDGED.FALSE, // String for GSI partition key ttl, location: event.location ? { lat: event.location.lat, @@ -1578,7 +1580,7 @@ async function hasUnacknowledgedAlert(deviceUid: string, alertType: string): Pro ExpressionAttributeValues: { ':device_uid': deviceUid, ':alert_type': alertType, - ':false': 'false', + ':false': ACKNOWLEDGED.FALSE, }, ScanIndexForward: false, // Most recent first }); diff --git a/songbird-infrastructure/lambda/api-journeys/index.ts b/songbird-infrastructure/lambda/api-journeys/index.ts index 52f42b6..a3fa98c 100644 --- a/songbird-infrastructure/lambda/api-journeys/index.ts +++ b/songbird-infrastructure/lambda/api-journeys/index.ts @@ -15,6 +15,7 @@ import { DynamoDBClient } from '@aws-sdk/client-dynamodb'; import { DynamoDBDocumentClient, QueryCommand, UpdateCommand, DeleteCommand, GetCommand, BatchWriteCommand } from '@aws-sdk/lib-dynamodb'; import { APIGatewayProxyEvent, APIGatewayProxyEventV2, APIGatewayProxyResult } from 'aws-lambda'; import { resolveDevice } from '../shared/device-lookup'; +import { parseIntParam } from '../shared/utils'; // Type for location point items from DynamoDB interface LocationPoint { @@ -146,7 +147,7 @@ async function listJourneys( headers: Record ): Promise { const status = queryParams.status; // 'active' | 'completed' | undefined (all) - const limit = parseInt(queryParams.limit || '50'); + const limit = parseIntParam(queryParams.limit, 50, 500); // Query all device_uids in parallel const queryPromises = deviceUids.map(async (deviceUid) => { @@ -473,9 +474,9 @@ async function getLocationHistory( queryParams: Record, headers: Record ): Promise { - const hours = parseInt(queryParams.hours || '24'); + const hours = parseIntParam(queryParams.hours, 24, 168); // max 168h = 1 week const source = queryParams.source; // 'gps' | 'cell' | 'triangulation' | undefined (all) - const limit = parseInt(queryParams.limit || '1000'); + const limit = parseIntParam(queryParams.limit, 1000, 5000); const cutoffTime = Date.now() - hours * 60 * 60 * 1000; diff --git a/songbird-infrastructure/lambda/api-telemetry/index.ts b/songbird-infrastructure/lambda/api-telemetry/index.ts index a13d4ed..82de2fd 100644 --- a/songbird-infrastructure/lambda/api-telemetry/index.ts +++ b/songbird-infrastructure/lambda/api-telemetry/index.ts @@ -15,6 +15,7 @@ import { DynamoDBClient } from '@aws-sdk/client-dynamodb'; import { DynamoDBDocumentClient, QueryCommand } from '@aws-sdk/lib-dynamodb'; import { APIGatewayProxyEvent, APIGatewayProxyResult } from 'aws-lambda'; import { resolveDevice } from '../shared/device-lookup'; +import { parseIntParam } from '../shared/utils'; const ddbClient = new DynamoDBClient({}); const docClient = DynamoDBDocumentClient.from(ddbClient); @@ -61,8 +62,8 @@ export const handler = async (event: APIGatewayProxyEvent): Promise `'${d.title.replace(/'/g, "''")}'`).join(','); - const excludePinned = pinnedTitles.length > 0 - ? `AND title NOT IN (${pinnedTitles})` + const titleParams = pinnedDocs.map((d, i) => ({ + name: `p${i}`, + value: { stringValue: d.title }, + })); + const excludePinned = titleParams.length > 0 + ? `AND title NOT IN (${titleParams.map(p => `:${p.name}`).join(',')})` : ''; const sql = ` SELECT title, content, doc_type, - 1 - (embedding <=> '${embeddingStr}'::vector) AS similarity + 1 - (embedding <=> :embedding::vector) AS similarity FROM analytics.rag_documents WHERE embedding IS NOT NULL AND pinned = FALSE ${excludePinned} - ORDER BY embedding <=> '${embeddingStr}'::vector - LIMIT ${topK} + ORDER BY embedding <=> :embedding::vector + LIMIT :limit `; const result = await rds.send(new ExecuteStatementCommand({ @@ -102,6 +105,11 @@ export async function retrieveRelevantContext( secretArn, database: databaseName, sql, + parameters: [ + { name: 'embedding', value: { stringValue: embeddingStr } }, + { name: 'limit', value: { longValue: topK } }, + ...titleParams, + ], })); const similarDocs: RetrievedDocument[] = (result.records || []).map(record => ({ diff --git a/songbird-infrastructure/lambda/shared/tracing.ts b/songbird-infrastructure/lambda/shared/tracing.ts index 1224206..07926aa 100644 --- a/songbird-infrastructure/lambda/shared/tracing.ts +++ b/songbird-infrastructure/lambda/shared/tracing.ts @@ -6,6 +6,7 @@ import { BatchSpanProcessor } from '@opentelemetry/sdk-trace-node'; import { OTLPTraceExporter } from '@opentelemetry/exporter-trace-otlp-proto'; const PHOENIX_HTTP_ENDPOINT = process.env.PHOENIX_HTTP_ENDPOINT || 'http://localhost:4318/v1/traces'; +const DEBUG_TRACING = process.env.DEBUG_TRACING === 'true'; let provider: NodeTracerProvider | null = null; let spanProcessor: BatchSpanProcessor | null = null; @@ -53,14 +54,14 @@ export function initializeTracing(serviceName: string): void { */ export async function flushSpans(): Promise { if (!spanProcessor) { - console.log('No span processor to flush'); + if (DEBUG_TRACING) console.log('No span processor to flush'); return; } try { - console.log('Flushing spans to Phoenix...'); + if (DEBUG_TRACING) console.log('Flushing spans to Phoenix...'); await spanProcessor.forceFlush(); - console.log('Spans flushed successfully'); + if (DEBUG_TRACING) console.log('Spans flushed successfully'); } catch (error) { console.error('Error flushing spans:', error); } @@ -74,7 +75,7 @@ export async function traceAsyncFn( ): Promise { const tracer = trace.getTracer('songbird'); const span = tracer.startSpan(name, { attributes, kind: spanKind }); - console.log(`Creating span: ${name}`); + if (DEBUG_TRACING) console.log(`Creating span: ${name}`); // Set this span as the active context so child spans nest under it const ctx = trace.setSpan(context.active(), span); @@ -82,7 +83,7 @@ export async function traceAsyncFn( try { const result = await context.with(ctx, () => fn(span)); span.setStatus({ code: SpanStatusCode.OK }); - console.log(`Span completed successfully: ${name}`); + if (DEBUG_TRACING) console.log(`Span completed successfully: ${name}`); return result; } catch (error) { span.setStatus({ @@ -94,6 +95,6 @@ export async function traceAsyncFn( throw error; } finally { span.end(); - console.log(`Span ended: ${name}`); + if (DEBUG_TRACING) console.log(`Span ended: ${name}`); } } diff --git a/songbird-infrastructure/lambda/shared/utils.ts b/songbird-infrastructure/lambda/shared/utils.ts new file mode 100644 index 0000000..014154b --- /dev/null +++ b/songbird-infrastructure/lambda/shared/utils.ts @@ -0,0 +1,20 @@ +/** + * Shared utility helpers for Lambda handlers + */ + +/** + * Safely parse an integer query-string parameter. + * + * Returns `defaultVal` when the value is missing, non-numeric, less than 1, + * or not finite — preventing NaN from propagating into DynamoDB Limit params. + * Optionally clamps the result to `max`. + */ +export function parseIntParam( + value: string | undefined, + defaultVal: number, + max?: number +): number { + const parsed = parseInt(value || String(defaultVal), 10); + if (!Number.isFinite(parsed) || parsed < 1) return defaultVal; + return max !== undefined ? Math.min(parsed, max) : parsed; +} diff --git a/songbird-infrastructure/lib/analytics-construct.ts b/songbird-infrastructure/lib/analytics-construct.ts index 7291030..56a2b34 100644 --- a/songbird-infrastructure/lib/analytics-construct.ts +++ b/songbird-infrastructure/lib/analytics-construct.ts @@ -237,16 +237,14 @@ export class AnalyticsConstruct extends Construct { this.chatHistoryTable.grantReadWriteData(this.chatQueryLambda); props.devicesTable.grantReadData(this.chatQueryLambda); - // Grant Bedrock access (includes Marketplace permissions for first-time model invocation) + // Grant Bedrock access scoped to the specific model used by this Lambda this.chatQueryLambda.addToRolePolicy(new iam.PolicyStatement({ effect: iam.Effect.ALLOW, actions: ['bedrock:InvokeModel'], - resources: ['*'], - })); - this.chatQueryLambda.addToRolePolicy(new iam.PolicyStatement({ - effect: iam.Effect.ALLOW, - actions: ['aws-marketplace:ViewSubscriptions', 'aws-marketplace:Subscribe'], - resources: ['*'], + resources: [ + `arn:aws:bedrock:*::foundation-model/anthropic.claude-3-5-sonnet-*`, + `arn:aws:bedrock:*::foundation-model/us.anthropic.claude-3-5-sonnet-*`, + ], })); // ==========================================================================