fix(infra): parseInt safety guards, remove duplicate code, JSON parse error handling, security hardening

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.

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`

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
+            }
         }
     }
 }

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
 }

songbird-infrastructure/lambda/analytics/chat-query.ts

@@ -464,35 +464,21 @@ export const handler = async (event: APIGatewayProxyEvent): Promise<APIGatewayPr
       };
     }
 
-    // Get user's accessible devices (from Cognito claims or database)
-    // If no devices specified, fetch all device serial numbers from Aurora
-    let deviceSerialNumbers = request.deviceSerialNumbers || [];
-
-    if (!deviceSerialNumbers || deviceSerialNumbers.length === 0) {
-      const devicesResult = await rds.send(new ExecuteStatementCommand({
-        resourceArn: CLUSTER_ARN,
-        secretArn: SECRET_ARN,
-        database: DATABASE_NAME,
-        sql: 'SELECT DISTINCT serial_number FROM analytics.devices',
-      }));
+    // Get user's accessible devices — must be supplied by the caller.
+    // Falling back to all devices would grant unrestricted data access.
+    const deviceSerialNumbers: string[] = request.deviceSerialNumbers?.length
+      ? request.deviceSerialNumbers
+      : [];
 
-      deviceSerialNumbers = (devicesResult.records || [])
-        .map(record => 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