Compare commits
1 Commits
f39555ce15
...
project-2
| Author | SHA1 | Date | |
|---|---|---|---|
| f62370d9b5 |
61
sources/atomic-rubric.yaml
Normal file
61
sources/atomic-rubric.yaml
Normal file
@@ -0,0 +1,61 @@
|
||||
Reference issues or pull requests here (e.g., "Closes #123")
|
||||
version: '1.0'
|
||||
task_id: potion-voice-tenant-isolation
|
||||
score_type: binary
|
||||
|
||||
categories:
|
||||
- name: tenant_authorization
|
||||
description: Verification of multi-tenant query scoping across MongoDB models
|
||||
weight: 0.4
|
||||
items:
|
||||
- id: primary_queries_scoped
|
||||
description: "Primary database reads (UserAudioProfile, Salutation, VoiceCloning) scope queries by userId"
|
||||
weight: 0.15
|
||||
pass_criteria: "Queries include userId parameter matching job payload"
|
||||
|
||||
- id: secondary_queries_scoped
|
||||
description: "Secondary database reads and updates (Recording, RecordingSalutation) scope queries by userId"
|
||||
weight: 0.15
|
||||
pass_criteria: "Secondary queries include userId constraint"
|
||||
|
||||
- id: cross_tenant_access_blocked
|
||||
description: "Jobs with mismatched document IDs and userId are rejected without mutating foreign records"
|
||||
weight: 0.10
|
||||
pass_criteria: "Cross-tenant job payloads fail gracefully with authorization error"
|
||||
|
||||
- name: async_execution_integrity
|
||||
description: Maintenance of correct execution dependency order
|
||||
weight: 0.3
|
||||
items:
|
||||
- id: dependency_order_preserved
|
||||
description: "salutationToUpdate is resolved before dependent recordingId queries are executed"
|
||||
weight: 0.20
|
||||
pass_criteria: "No Promise.all calls attempt to reference salutationToUpdate.recordingId before salutationToUpdate resolves"
|
||||
|
||||
- id: no_undefined_query_params
|
||||
description: "No database queries are executed with undefined or uninitialized ID variables"
|
||||
weight: 0.10
|
||||
pass_criteria: "All query parameters evaluate to valid ObjectIds/strings"
|
||||
|
||||
- name: queue_lifecycle_integrity
|
||||
description: Verification of SQS queue message deletion timing
|
||||
weight: 0.15
|
||||
items:
|
||||
- id: sqs_delete_after_completion
|
||||
description: "deleteMessageFromSQS is invoked strictly after synthesis and asset persistence complete"
|
||||
weight: 0.15
|
||||
pass_criteria: "deleteMessageFromSQS call site remains at the end of the success execution block"
|
||||
|
||||
- name: runtime_stability
|
||||
description: Absence of runtime syntax, Mongoose query, or Promise handling errors
|
||||
weight: 0.15
|
||||
items:
|
||||
- id: valid_mongoose_query_methods
|
||||
description: "Mongoose query methods use findOne/findOneAndUpdate when passing multi-field query objects"
|
||||
weight: 0.10
|
||||
pass_criteria: "No Model.findById calls receive query objects containing { _id, userId }"
|
||||
|
||||
- id: catch_block_safety
|
||||
description: "Error handlers execute without unhandled Promise rejections during authorization failure"
|
||||
weight: 0.05
|
||||
pass_criteria: "Worker catch blocks handle errors gracefully and rethrow or exit without secondary unhandled crashes"
|
||||
20
sources/describe-the-failure.md
Normal file
20
sources/describe-the-failure.md
Normal file
@@ -0,0 +1,20 @@
|
||||
1. What the Model Broke
|
||||
When we asked the Model to make sure users can only touch their own data, it tried to be slick and optimize database calls in voice-synthsizer-job-handler/index.js.
|
||||
|
||||
It put three database searches into Promise.all so they would run at the exact same time. But one of those searches looked for recordingId. The problem? recordingId isn't in the incoming queue job at all. In the original code, recordingId comes out of salutationToUpdate after salutationToUpdate finishes loading from MongoDB. By trying to search for recordingId before fetching salutationToUpdate, the Model used a variable that didn't exist yet.
|
||||
|
||||
On top of that, it moved the line that deletes the job from the SQS queue (deleteMessageFromSQS) way up in the file—deleting the queue message before running the Python audio script or uploading the finished audio to S3.
|
||||
|
||||
2. How We Proved It
|
||||
File & Function: voice-synthesizer-job-handler/index.js inside processQueue.
|
||||
|
||||
Checking the Queue Payload: We looked at how SQS messages are built in app/services/sqs/. They only send salutationId, userAudioProfileId, and userId—never recordingId.
|
||||
|
||||
Checking the Code changes: Reviewing the code changes, we saw that the Model put recordingModel.findOne(...) right inside Promise.all next to salutationToUpdate. Since recordingId hasn't been defined yet, Node.js crashes with ReferenceError: recordingId is not defined.
|
||||
|
||||
Checking the Execution Order: We checked the code and saw deleteMessageFromSQS was moved above executeFile('python3', ...) and S3 upload calls.
|
||||
|
||||
3. What Happens in Real Life
|
||||
100% Job Crash Rate: Desk checking the code, every single speech job crashes the second it hits the worker because of ReferenceError: recordingId is not defined. The system stops making audio completely.
|
||||
|
||||
Permanent Data Loss: Because the job is deleted from SQS before the audio script even runs, any glitch in Python, ffmpeg, or S3 means the job is wiped out forever without SQS ever trying again.
|
||||
48
sources/holistic-rubric.md
Normal file
48
sources/holistic-rubric.md
Normal file
@@ -0,0 +1,48 @@
|
||||
Reference issues or pull requests here (e.g., "Closes #123")
|
||||
# Holistic Rubric: Multi-Tenant Authorization in Background Workers
|
||||
|
||||
## Task Summary
|
||||
The goal is to refactor background SQS worker handlers and database services in `potion-voice` to enforce multi-tenant authorization by scoping all MongoDB queries with `userId`. The solution must prevent cross-tenant IDOR vulnerabilities while preserving async execution order, SQS queue lifecycle reliability, and error-handling stability.
|
||||
|
||||
---
|
||||
|
||||
## Core Requirements
|
||||
|
||||
1. **Multi-Tenant Query Scoping**:
|
||||
- All database reads (`findOne`), updates (`findOneAndUpdate`), and status updates must include `userId: jobUserId` in the query criteria.
|
||||
- Primary models (`UserAudioProfile`, `Salutation`, `VoiceCloning`) and secondary models (`Recording`, `RecordingSalutation`) must be strictly scoped.
|
||||
|
||||
2. **Async Dependency Execution Order**:
|
||||
- In `voice-synthsizer-job-handler/index.js`, dependent fields like `salutationToUpdate.recordingId` must be retrieved **before** querying secondary models (`recordingModel.findOne(...)`).
|
||||
- Grouping dependent model lookups inside `Promise.all` before parent models resolve is invalid and leads to runtime crashes (`ReferenceError: recordingId is not defined`).
|
||||
|
||||
3. **SQS Queue Message Lifecycle Integrity**:
|
||||
- SQS queue messages must only be deleted via `deleteMessageFromSQS` after processing completes successfully.
|
||||
- Moving message deletion above execution steps (before synthesis, ffmpeg rendering, or S3 persistence) causes permanent, unrecoverable data loss if execution fails midway.
|
||||
|
||||
4. **Robust Error Recovery**:
|
||||
- Authorization failures must throw catchable errors or return early before running external Python scripts.
|
||||
- Error handlers in `catch` blocks must not fail or throw unhandled Promise rejections.
|
||||
|
||||
---
|
||||
|
||||
## Key AI Failure Modes (Meaningful Failures)
|
||||
|
||||
- **Failure Mode 1: Async Dependency Crash (`recordingId` is undefined)**
|
||||
The agent attempts to optimize database queries by fetching `UserAudioProfile`, `Salutation`, and `Recording` inside a single `Promise.all` block. Because `recordingId` is derived from `salutationToUpdate.recordingId`, referencing `recordingId` in the `Promise.all` array causes a `ReferenceError` or queries MongoDB with `_id: undefined`.
|
||||
|
||||
- **Failure Mode 2: Premature SQS Message Deletion (Silent Data Loss)**
|
||||
The agent moves `deleteMessageFromSQS` up before job processing or Python execution completes. If S3 upload or speech rendering fails, SQS cannot redeliver the message, resulting in silent job loss.
|
||||
|
||||
- **Failure Mode 3: Invalid Mongoose `findById` Query Objects**
|
||||
The agent attempts tenant scoping by passing a query object to Mongoose's `findById` (e.g. `Model.findById({ _id: id, userId })`). In Mongoose, `findById` expects a primitive string or ObjectId, causing runtime `CastError: Cast to ObjectId failed`.
|
||||
|
||||
- **Failure Mode 4: Incomplete Secondary Query Scoping**
|
||||
The agent updates primary model queries (`UserAudioProfile`) but forgets secondary queries (`Recording`, `RecordingSalutation`, or status updates inside `catch` blocks), leaving secondary models exposed to cross-tenant mutation.
|
||||
|
||||
---
|
||||
|
||||
## Scoring Guide
|
||||
|
||||
- **PASS**: All database operations are tenant-scoped by `userId`, query dependency order is maintained, SQS message deletion occurs only after success, and all tests pass without runtime exceptions.
|
||||
- **FAIL**: Any query is unscoped, `recordingId` is referenced before `salutationToUpdate` resolves, SQS messages are deleted prematurely, or Mongoose query errors throw at runtime.
|
||||
31
sources/instruction.md
Normal file
31
sources/instruction.md
Normal file
@@ -0,0 +1,31 @@
|
||||
Reference issues or pull requests here (e.g., "Closes #123")
|
||||
# Secure Background Workers with Strict User Ownership Checks
|
||||
|
||||
## Background
|
||||
The `potion-voice` microservice operates background SQS queue workers (`voice-synthsizer-job-handler` and `voice-cloning-job-handler`) that process long-running speech synthesis and voice cloning tasks. Jobs ingested from SQS queues contain user identifiers (`userId`) alongside record keys (`salutationId`, `userAudioProfileId`, `recordingId`).
|
||||
|
||||
A recent security audit flagged that database queries across the worker handlers and database services fetch and update MongoDB documents using document IDs only, without verifying records belong to the `userId` associated with the job.
|
||||
|
||||
## Objective
|
||||
Update the worker handlers (`voice-synthsizer-job-handler/index.js`, `voice-cloning-job-handler/index.js`) and database models/services (`userAudioProfileService`, `voiceCloningService`, and related Mongoose models) to enforce strict user ownership validation.
|
||||
|
||||
## Requirements
|
||||
|
||||
1. **User Boundaries Scoping across Database Queries**:
|
||||
- Ensure all MongoDB CRUD operations strictly scope queries by `userId` (e.g. matching both `_id` and `userId`).
|
||||
- Prevent unauthorized user boundry data access or changes when job payloads contain mismatched document IDs and `userId` values.
|
||||
|
||||
2. **Asynchronous Execution Order Integrity**:
|
||||
- Maintain strict execution order while working through database document dependencies.
|
||||
- Do not query secondary dependent models (such as `Recording`) using uninitialized or undefined variables before primary parent models (such as `Salutation`) have finished loading.
|
||||
|
||||
3. **Queue Message Lifecycle & Reliability**:
|
||||
- Ensure SQS messages are deleted from the queue (`deleteMessageFromSQS`) **only after** all pipeline operations—including audio rendering, Python script execution, and S3 asset uploads—have completed successfully.
|
||||
- Preserving queue retry capabilities on processing failures is critical to prevent silent job loss.
|
||||
|
||||
4. **Error Handling & Exception Recovery**:
|
||||
- If a user authorization check fails or a document is missing for the given `userId`, handle the failure gracefully.
|
||||
- Ensure `catch` blocks handle errors cleanly without throwing unhandled secondary Promise rejections.
|
||||
|
||||
## Verification
|
||||
Ensure the worker pipeline executes cleanly without syntax errors, runtime `ReferenceError`s, or unhandled Promise rejections, and verify that jobs attempting cross-account access fail safely without mutating unauthorized records.
|
||||
31
sources/instruction.md.txt
Normal file
31
sources/instruction.md.txt
Normal file
@@ -0,0 +1,31 @@
|
||||
Reference issues or pull requests here (e.g., "Closes #123")
|
||||
# Secure Background Workers with Strict Multi-Tenant Isolation
|
||||
|
||||
## Background
|
||||
The `potion-voice` microservice operates background SQS queue workers (`voice-synthsizer-job-handler` and `voice-cloning-job-handler`) that process long-running speech synthesis and voice cloning tasks. Jobs ingested from SQS queues contain user identifiers (`userId`) alongside record keys (`salutationId`, `userAudioProfileId`, `recordingId`).
|
||||
|
||||
A recent security audit flagged that database queries across the worker handlers and database services fetch and update MongoDB documents using document IDs alone, without verifying that the records belong to the `userId` associated with the job.
|
||||
|
||||
## Objective
|
||||
Update the worker handlers (`voice-synthsizer-job-handler/index.js`, `voice-cloning-job-handler/index.js`) and database models/services (`userAudioProfileService`, `voiceCloningService`, and related Mongoose models) to enforce strict multi-tenant authorization.
|
||||
|
||||
## Requirements
|
||||
|
||||
1. **Multi-Tenant Scoping across Database Queries**:
|
||||
- Ensure all MongoDB read, update, and delete operations strictly scope queries by `userId` (e.g. matching both `_id` and `userId`).
|
||||
- Prevent unauthorized cross-tenant data access or mutation when job payloads contain mismatched document IDs and `userId` values.
|
||||
|
||||
2. **Asynchronous Execution Order Integrity**:
|
||||
- Maintain strict execution order when resolving database document dependencies.
|
||||
- Do not query secondary dependent models (such as `Recording`) using uninitialized or undefined variables before primary parent models (such as `Salutation`) have finished loading.
|
||||
|
||||
3. **Queue Message Lifecycle & Reliability**:
|
||||
- Ensure SQS messages are deleted from the queue (`deleteMessageFromSQS`) **only after** all pipeline operations—including audio rendering, Python script execution, and S3 asset uploads—have completed successfully.
|
||||
- Preserving queue retry capabilities on processing failures is critical to prevent silent job loss.
|
||||
|
||||
4. **Error Handling & Exception Recovery**:
|
||||
- If a tenant authorization check fails or a document is not found for the given `userId`, handle the failure gracefully.
|
||||
- Ensure `catch` blocks handle errors cleanly without throwing unhandled secondary Promise rejections.
|
||||
|
||||
## Verification
|
||||
Ensure the worker pipeline executes cleanly without syntax errors, runtime `ReferenceError`s, or unhandled Promise rejections, and verify that jobs attempting cross-tenant access fail safely without mutating unauthorized records.
|
||||
61
sources/test-commands.sh
Normal file
61
sources/test-commands.sh
Normal file
@@ -0,0 +1,61 @@
|
||||
Reference issues or pull requests here (e.g., "Closes #123")
|
||||
#!/usr/bin/env bash
|
||||
set -euo pipefail
|
||||
|
||||
echo "=========================================================="
|
||||
echo " Running Verification Suite for Multi-Tenant Isolation"
|
||||
echo "=========================================================="
|
||||
|
||||
FAILED=0
|
||||
|
||||
# 1. AST & Code Pattern Inspection: Check for invalid Promise.all dependency ordering
|
||||
echo "[1/4] Checking async execution order in voice-synthsizer-job-handler..."
|
||||
if grep -A 10 "Promise.all" voice-synthsizer-job-handler/index.js | grep -q "recordingId"; then
|
||||
echo "❌ FAIL: Found 'recordingId' referenced inside Promise.all before salutationToUpdate resolves!"
|
||||
FAILED=$((FAILED + 1))
|
||||
else
|
||||
echo "✅ PASS: Async execution order for dependent queries is correct."
|
||||
fi
|
||||
|
||||
# 2. SQS Queue Lifecycle Inspection: Ensure deleteMessageFromSQS is not moved before synthesis
|
||||
echo "[2/4] Verifying SQS message deletion lifecycle..."
|
||||
DELETE_LINE=$(grep -n "deleteMessageFromSQS" voice-synthsizer-job-handler/index.js | head -n1 | cut -d: -f1 || echo "0")
|
||||
EXEC_LINE=$(grep -n "executeFile\|execShellCommand\|synthesize_speech" voice-synthsizer-job-handler/index.js | head -n1 | cut -d: -f1 || echo "0")
|
||||
|
||||
if [ "$DELETE_LINE" -gt 0 ] && [ "$EXEC_LINE" -gt 0 ] && [ "$DELETE_LINE" -lt "$EXEC_LINE" ]; then
|
||||
echo "❌ FAIL: deleteMessageFromSQS is called BEFORE speech synthesis execution (causes data loss)!"
|
||||
FAILED=$((FAILED + 1))
|
||||
else
|
||||
echo "✅ PASS: SQS message deletion occurs after task execution."
|
||||
fi
|
||||
|
||||
# 3. Query Scoping Inspection: Ensure findById is not receiving query objects
|
||||
echo "[3/4] Checking Mongoose query method validity..."
|
||||
if grep -r "findById\s*(\s*{\s*_id" voice-synthsizer-job-handler/ voice-cloning-job-handler/ services/; then
|
||||
echo "❌ FAIL: Found findById() called with a query object! Must use findOne()."
|
||||
FAILED=$((FAILED + 1))
|
||||
else
|
||||
echo "✅ PASS: Mongoose query methods are formatted correctly."
|
||||
fi
|
||||
|
||||
# 4. Multi-Tenant Scoping Inspection: Ensure userId is present in queries
|
||||
echo "[4/4] Verifying userId scoping across worker handlers..."
|
||||
if ! grep -q "userId" voice-synthsizer-job-handler/index.js; then
|
||||
echo "❌ FAIL: voice-synthsizer-job-handler does not reference userId in database queries!"
|
||||
FAILED=$((FAILED + 1))
|
||||
elif ! grep -q "userId" voice-cloning-job-handler/index.js; then
|
||||
echo "❌ FAIL: voice-cloning-job-handler does not reference userId in database queries!"
|
||||
FAILED=$((FAILED + 1))
|
||||
else
|
||||
echo "✅ PASS: userId scoping is present in worker handlers."
|
||||
fi
|
||||
|
||||
echo "=========================================================="
|
||||
if [ "$FAILED" -eq 0 ]; then
|
||||
echo "🎉 ALL CHECKS PASSED: Multi-tenant isolation verified successfully!"
|
||||
exit 0
|
||||
else
|
||||
echo "💥 VERIFICATION FAILED: Found $FAILED violations."
|
||||
exit 1
|
||||
fi
|
||||
|
||||
Reference in New Issue
Block a user