|
|
|
|
@@ -1,105 +1,108 @@
|
|
|
|
|
# Holistic Rubric: Multi-Tenant Database Query Isolation in Background Workers
|
|
|
|
|
# Holistic Rubric: Multi-Tenant Authorization in Background Workers
|
|
|
|
|
|
|
|
|
|
## Task Context
|
|
|
|
|
Backend background workers handling voice synthesis (`voice-synthsizer-job-handler/index.js`) and voice cloning (`voice-cloning-job-handler/index.js`) must be audited and refactored to enforce strict multi-tenant data isolation. All database operations across primary and secondary models must ensure users can only access or modify records belonging to their authenticated identity, preventing unauthorized cross-tenant data access while maintaining system stability and error resilience.
|
|
|
|
|
### Task Context
|
|
|
|
|
The backend background workers (`voice-synthsizer-job-handler` and `voice-cloning-job-handler`) and database service wrappers in `potion-voice` currently retrieve MongoDB records using document IDs without verifying multi-tenant user ownership. The task requires auditing and refactoring all database query entry points across both workers to enforce strict `userId` scoping, preventing cross-tenant data access (IDOR) while preserving asynchronous execution order and queue message safety.
|
|
|
|
|
|
|
|
|
|
---
|
|
|
|
|
|
|
|
|
|
## Ground Truth & Technical Requirements
|
|
|
|
|
### Ground Truth
|
|
|
|
|
1. **Queue Message Context & User Identity Provenance**:
|
|
|
|
|
- The task prompt specifies that worker processes must scope database queries by the job's `userId`.
|
|
|
|
|
- In the shipped baseline repository, worker handlers destructure job properties from incoming SQS messages (`JSON.parse(response.Messages[0].Body)`), but baseline code omits `userId` destructuring. No SQS producer code or JSON sample message fixtures are committed in the local workspace repository.
|
|
|
|
|
- To satisfy multi-tenant isolation, trial agents must obtain `userId` directly from the incoming job message payload/context (e.g., `job.userId` or `job._doc.userId`). If a message lacks a valid `userId`, the worker must reject processing cleanly. An unscoped database lookup (such as `UserAudioProfile.findById(job.userAudioProfileId)`) cannot be used to "discover" or establish an owner `userId`.
|
|
|
|
|
|
|
|
|
|
### 1. User Identity Provenance & Trusted Tenant Context
|
|
|
|
|
* **Trusted Identity Source**: The authenticated `userId` must originate directly from the incoming SQS job payload/context (e.g., `job.userId` or `job._doc.userId`).
|
|
|
|
|
* **Unscoped Lookup Security Bypass**: An unscoped database lookup (such as calling `UserAudioProfile.findById(job.userAudioProfileId)` without `userId` scoping) cannot be used to establish or "discover" a trusted owner `userId`. Performing an unscoped lookup to retrieve a tenant ID from an unverified record ID represents a multi-tenant security vulnerability.
|
|
|
|
|
2. **Database Relationships & Key Dependencies**:
|
|
|
|
|
- `UserAudioProfile` represents user-owned cloned voice profiles (`_id`, `userId`, `status`).
|
|
|
|
|
- `VoiceCloning` tracks voice model training jobs (`_id`, `userId`, `userAudioProfileId`, `status`).
|
|
|
|
|
- `RecordingSalutation` links a personalized salutation (`salutationId`) to a parent recording (`recordingId`). `recordingId` is optional in schema (`required: false`).
|
|
|
|
|
- `Salutation` stores generated audio greetings (`_id`, `userId`, `userAudioProfileId`).
|
|
|
|
|
- `Job` tracks downstream video synthesis tasks (`_id`, `userId`, `salutationId`, `status`).
|
|
|
|
|
- `Recording` represents dynamic video templates (`_id`, `userId`, `status`).
|
|
|
|
|
|
|
|
|
|
### 2. Disambiguated Query Scoping Rules (`_id` vs. Tenant-Wide)
|
|
|
|
|
* **Record ID Lookups and Updates (`findById`, `findOne`, `findOneAndUpdate`, `updateOne`, `deleteOne`)**:
|
|
|
|
|
* When querying or updating a specific record by its ID, queries MUST combine the document ID and user identity in the search conditions: `{ _id: recordId, userId }` (or `{ _id: recordId, userId, deleted: false }`).
|
|
|
|
|
* **Tenant-Wide and Collection-Level Queries (`find`, `updateMany`, Profile Lookups)**:
|
|
|
|
|
* When querying or updating collections for a user without a specific target document ID (such as listing all user recordings, fetching a user's audio profile by `userId`, or updating all user jobs), queries MUST filter by `{ userId }` (along with operational filters like `{ deleted: false }`).
|
|
|
|
|
* `_id` is **NOT** required or expected on collection-level or tenant-wide queries where a single document `_id` is not part of the search criteria.
|
|
|
|
|
3. **Mongoose Method & Parameter Signatures**:
|
|
|
|
|
- `findOneAndUpdate` accepts positional arguments: `findOneAndUpdate(conditions, update, options, callback)`.
|
|
|
|
|
- To enforce tenant isolation, argument 1 (`conditions`) MUST contain the `userId` filter alongside the document ID: `{ _id: recordId, userId, deleted: false }`.
|
|
|
|
|
- Placing tenant filters into argument 3 (`options`) or passing extra arguments leaves argument 1 (`conditions`) unscoped, allowing query execution against foreign tenant records.
|
|
|
|
|
- For collection-level or multi-record operations (`find`, `updateMany`, `removeMany`), queries must filter by `{ userId, deleted: false }`. `_id` is NOT required or expected when a specific document ID is not part of the search criteria.
|
|
|
|
|
|
|
|
|
|
### 3. Mongoose `findOneAndUpdate` Parameter Signature Accuracy
|
|
|
|
|
* Mongoose `findOneAndUpdate` accepts positional parameters: `findOneAndUpdate(conditions, update, options, callback)`.
|
|
|
|
|
* **Argument 1 (`conditions`) Requirement**: The query filter in argument 1 (`conditions`) MUST explicitly contain the `userId` filter (e.g., `{ _id: recordId, userId }`).
|
|
|
|
|
* **Misplacement Flaw**: Placing tenant filters into argument 3 (`options`) or subsequent arguments under the assumption that Mongoose treats `options` as query conditions leaves argument 1 (`conditions`) unscoped by `userId`.
|
|
|
|
|
* **Extra Arguments Flaw**: Passing 5 or more arguments exceeds Mongoose's API signature (`conditions, update, options, callback`), causing Mongoose to ignore extra arguments. Passing 5 arguments alone does not automatically make argument 1 unscoped; the security defect is specifically leaving argument 1 (`conditions`) without `userId` scoping.
|
|
|
|
|
4. **SQS Queue Lifecycle & Execution Order**:
|
|
|
|
|
- In baseline `voice-cloning-job-handler/index.js` and `voice-synthsizer-job-handler/index.js`, `sqs.deleteMessageFromSQS(sqsQueueUrl, receiptHandle)` is invoked at worker entry.
|
|
|
|
|
- Re-positioning or retaining message deletion at queue entry is a pre-existing queue behavior in baseline code. Solutions that enforce strict `userId` query scoping satisfy the primary security request.
|
|
|
|
|
|
|
|
|
|
### 4. Error Handling & Foreign Tenant Rejection
|
|
|
|
|
* **Authorization Failures**: When a job message references a record that fails user ownership verification or authorization checks, background workers must halt processing cleanly without mutating foreign tenant records.
|
|
|
|
|
* **Safe Error Status Updates**: Status updates written upon error or rejection must be scoped exclusively to the authenticated user's own record (`{ _id: jobId, userId }`), ensuring error statuses are recorded safely without modifying unauthorized documents.
|
|
|
|
|
* **SQS Queue Reliability**: Background job processing must handle exceptions gracefully to prevent unhandled runtime crashes or infinite loop re-runs.
|
|
|
|
|
5. **Error & Foreign Tenant Rejection Safety**:
|
|
|
|
|
- When a job is rejected due to missing/invalid authorization or foreign tenant mismatch, background workers must halt execution cleanly without mutating foreign tenant records.
|
|
|
|
|
- Status updates to `'error'` or `'failed'` must be scoped exclusively to records owned by the authenticated user (`{ _id, userId }`).
|
|
|
|
|
|
|
|
|
|
---
|
|
|
|
|
|
|
|
|
|
## Evaluation Across Standard Dimensions
|
|
|
|
|
### Key AI Failure Modes
|
|
|
|
|
|
|
|
|
|
### Narrow Correctness
|
|
|
|
|
* **PASS**:
|
|
|
|
|
* All database operations on primary and secondary models (`UserAudioProfile`, `VoiceCloning`, `Salutation`, `Recording`, `Job`, `RecordingSalutation`) properly enforce `userId` scoping.
|
|
|
|
|
* Record-specific lookups combine record ID and user identity (`{ _id, userId }`), while tenant-wide queries filter by `{ userId }`.
|
|
|
|
|
* Trusted `userId` is obtained from the job payload context rather than via an initial unscoped database lookup.
|
|
|
|
|
* **FAIL**:
|
|
|
|
|
* Any database query or update operation executes using only a record identifier (`_id`) without `userId` scoping.
|
|
|
|
|
* Code performs an unscoped database lookup to fetch `userId` before enforcing tenant checks.
|
|
|
|
|
* Argument 1 (`conditions`) of `findOneAndUpdate` is left unscoped by `userId`.
|
|
|
|
|
1. **Missing Utility Module Startup Crash (`MODULE_NOT_FOUND`)**:
|
|
|
|
|
- The agent adds import statements like `const { requireUserId, tenantFilter } = require('../worker_tenant')` in `voice-cloning-job-handler/index.js` or `voice-synthsizer-job-handler/index.js` without creating `worker_tenant.js` or `worker_tenant/index.js`. Node.js throws `Error: Cannot find module '../worker_tenant'`, causing a 100% startup crash for worker instances.
|
|
|
|
|
|
|
|
|
|
### Broader Correctness
|
|
|
|
|
* **PASS**:
|
|
|
|
|
* Asynchronous dependency ordering is preserved (e.g., parent documents resolve before dependent child lookups execute).
|
|
|
|
|
* Error handling cleanly catches authorization rejections and runtime exceptions without crashing the worker process or corrupting cross-tenant data.
|
|
|
|
|
* **FAIL**:
|
|
|
|
|
* Introduces runtime execution crashes, such as referencing undefined variables in concurrent `Promise.all` blocks.
|
|
|
|
|
* Attempts to update foreign tenant records during error recovery.
|
|
|
|
|
2. **Unscoped Mongoose `findOneAndUpdate` Query Conditions (Security Bypass)**:
|
|
|
|
|
- The agent updates service methods (e.g., `user_audio_profile_service.js`, `voice_cloning_service.js`, `salutation_service.js`, `job_service.js`) by passing `{ _id: data._id }` as argument 1 (`conditions`), while placing tenant scoping in argument 3 (`options`). Argument 1 remains unscoped by `userId`, bypassing multi-tenant security boundaries.
|
|
|
|
|
|
|
|
|
|
### Common Sense
|
|
|
|
|
* **PASS**:
|
|
|
|
|
* Code changes directly address multi-tenant database isolation without introducing unnecessary architectural refactors or unneeded external module dependencies.
|
|
|
|
|
* **FAIL**:
|
|
|
|
|
* Introduces references to non-existent utility modules (e.g. `require('../worker_tenant')` without creating the file), causing immediate `MODULE_NOT_FOUND` startup crashes.
|
|
|
|
|
* Deletes required model imports or breaks existing Mongoose API call structures.
|
|
|
|
|
3. **Async Dependency Execution Crash (`ReferenceError: recordingId is not defined`)**:
|
|
|
|
|
- The agent groups dependent database lookups into a concurrent `Promise.all` block before parent documents resolve (e.g., trying to read `Recording` using `recordingId` before `salutationToUpdate` resolves `recordingId`). Referencing uninitialized variables inside `Promise.all` throws `ReferenceError: recordingId is not defined`.
|
|
|
|
|
|
|
|
|
|
### Verification & Thoroughness
|
|
|
|
|
* **PASS**:
|
|
|
|
|
* Refactoring systematically covers all query entry points in both `voice-synthsizer-job-handler/index.js` and `voice-cloning-job-handler/index.js`.
|
|
|
|
|
* **FAIL**:
|
|
|
|
|
* Refactors queries in one worker handler while leaving queries in the second handler unscoped.
|
|
|
|
|
4. **Unscoped Profile Lookup to Discover Owner (Circular Auth Bypass)**:
|
|
|
|
|
- The agent fetches an unverified profile via `UserAudioProfile.findById(job.userAudioProfileId)` without `userId` scoping, and then reads `profile.userId` to establish the job's owner. Because an unverified document ID determines the owner, a malicious SQS payload can specify any profile ID to manipulate foreign tenant data.
|
|
|
|
|
|
|
|
|
|
### Integrity
|
|
|
|
|
* **PASS**:
|
|
|
|
|
* Preserves data boundaries between tenants and maintains accurate record attributes during update operations.
|
|
|
|
|
* **FAIL**:
|
|
|
|
|
* Bypasses security filters by placing scoping rules in invalid parameter positions or suppressing authorization errors.
|
|
|
|
|
|
|
|
|
|
### Persistence
|
|
|
|
|
* **PASS**:
|
|
|
|
|
* Resolves multi-tenant isolation across complex nested queries and secondary model dependencies.
|
|
|
|
|
* **FAIL**:
|
|
|
|
|
* Abandons query refactoring when handling secondary model relationships or asynchronous callbacks.
|
|
|
|
|
|
|
|
|
|
### Communication
|
|
|
|
|
* **PASS**:
|
|
|
|
|
* Code modifications and comments clearly explain the multi-tenant query scoping logic.
|
|
|
|
|
* **FAIL**:
|
|
|
|
|
* Obfuscates query conditions or leaves misleading parameters in database calls.
|
|
|
|
|
|
|
|
|
|
### Thought Partnership
|
|
|
|
|
* **PASS**:
|
|
|
|
|
* Implements clean, maintainable, and standard Mongoose query scoping patterns aligned with Node.js best practices.
|
|
|
|
|
* **FAIL**:
|
|
|
|
|
* Applies anti-patterns that complicate query execution or introduce subtle race conditions.
|
|
|
|
|
5. **Foreign Tenant Record Mutation on Rejection**:
|
|
|
|
|
- Upon encountering an unauthorized job message, the agent executes status updates (`status: 'error'`) without `userId` filters, mutating database records belonging to an unauthenticated foreign tenant.
|
|
|
|
|
|
|
|
|
|
---
|
|
|
|
|
|
|
|
|
|
## Qualitative Heavy Penalties
|
|
|
|
|
### Grading Dimensions
|
|
|
|
|
|
|
|
|
|
1. **Unscoped Database Query Penalty** (*Target Dimension: Narrow Correctness*):
|
|
|
|
|
* **Trigger**: Any database lookup, update, or deletion executed without `userId` scoping in query conditions (including unscoped initial profile lookups or unscoped `findOneAndUpdate` condition arguments).
|
|
|
|
|
* **Nearest Acceptable Behavior**: All single-record queries use `{ _id, userId }` and all tenant-wide/profile queries filter by `{ userId }` using trusted job context.
|
|
|
|
|
#### Narrow Correctness
|
|
|
|
|
- **PASS**: Background worker processes (`voice-synthsizer-job-handler` and `voice-cloning-job-handler`) execute cleanly without throwing `MODULE_NOT_FOUND`, `ReferenceError`, or Mongoose `CastError` exceptions.
|
|
|
|
|
- **FAIL**: Code crashes on startup due to missing files/imports, throws unhandled `ReferenceError` exceptions due to improper `Promise.all` concurrency, or fails syntax validation.
|
|
|
|
|
|
|
|
|
|
2. **Missing Utility Module Import Penalty** (*Target Dimension: Common Sense*):
|
|
|
|
|
* **Trigger**: Importing non-existent files or utilities that trigger `MODULE_NOT_FOUND` startup crashes.
|
|
|
|
|
* **Nearest Acceptable Behavior**: Using inline helper functions or properly creating and exporting any new utility modules required by the implementation.
|
|
|
|
|
#### Broader Correctness
|
|
|
|
|
- **PASS**: All database lookups and update operations across models (`UserAudioProfile`, `VoiceCloning`, `Salutation`, `Job`, `Recording`) explicitly enforce `userId` scoping, preventing cross-tenant data access.
|
|
|
|
|
- **FAIL**: Database queries in worker handlers or service wrappers rely solely on document IDs (`_id`) without `userId` conditions, or place tenant filters into Mongoose `options` parameters instead of `conditions`.
|
|
|
|
|
|
|
|
|
|
3. **Async Dependency Execution Crash Penalty** (*Target Dimension: Broader Correctness*):
|
|
|
|
|
* **Trigger**: Structuring dependent queries concurrently in `Promise.all` before parent IDs are resolved, causing `ReferenceError` runtime crashes.
|
|
|
|
|
* **Nearest Acceptable Behavior**: Awaiting parent document resolution sequentially before passing resolved foreign keys into dependent query conditions.
|
|
|
|
|
#### Common Sense
|
|
|
|
|
- **PASS**: Asynchronous execution order respects document dependencies, resolving parent records before dependent child queries.
|
|
|
|
|
- **FAIL**: Grouping sequential dependent queries into concurrent `Promise.all` blocks before child IDs are resolved.
|
|
|
|
|
|
|
|
|
|
#### Verification & Thoroughness
|
|
|
|
|
- **PASS**: The agent systematically audits and refactors query entry points across both worker handlers (`voice-synthsizer-job-handler` and `voice-cloning-job-handler`) and service wrappers.
|
|
|
|
|
- **FAIL**: Refactoring only one worker handler while leaving the second worker handler completely unscoped.
|
|
|
|
|
|
|
|
|
|
#### Integrity
|
|
|
|
|
- **PASS**: The agent implements genuine query scoping without fabricating test passes or misrepresenting security coverage.
|
|
|
|
|
- **FAIL**: Claiming multi-tenant isolation is complete while leaving raw Mongoose queries unscoped in service wrappers.
|
|
|
|
|
|
|
|
|
|
#### Persistence
|
|
|
|
|
- **PASS**: The agent works through baseline execution details and completes the multi-file refactoring across handlers and services.
|
|
|
|
|
- **FAIL**: Stopping after editing a single file or asking unnecessary questions when the codebase provides all required context.
|
|
|
|
|
|
|
|
|
|
#### Communication
|
|
|
|
|
- **PASS**: The final response clearly explains the audited worker files, service wrappers, and multi-tenant scoping logic added.
|
|
|
|
|
- **FAIL**: Providing inaccurate explanations of Mongoose query execution or misleading statements regarding worker stability.
|
|
|
|
|
|
|
|
|
|
#### Thought Partnership
|
|
|
|
|
- **PASS**: The agent identifies all primary and secondary model entry points across both workers and applies consistent `userId` query scoping.
|
|
|
|
|
- **FAIL**: Implementing partial scoping that breaks baseline processing or introducing uncreated utility dependencies.
|
|
|
|
|
|
|
|
|
|
---
|
|
|
|
|
|
|
|
|
|
### Heavy Penalties
|
|
|
|
|
|
|
|
|
|
1. **Unscoped Database Queries (Multi-Tenant Security Vulnerability)**:
|
|
|
|
|
- *Triggers when*: Any document lookup or update operation in `voice-synthsizer-job-handler`, `voice-cloning-job-handler`, or service wrappers performs database operations without `userId` scoping.
|
|
|
|
|
- *Target Dimension*: **Broader Correctness**.
|
|
|
|
|
- *Nearest Acceptable Behavior*: All single-record queries enforce `{ _id, userId, deleted: false }` and multi-record operations enforce `{ userId, deleted: false }`.
|
|
|
|
|
|
|
|
|
|
2. **Missing Utility Module Startup Crash (`MODULE_NOT_FOUND`)**:
|
|
|
|
|
- *Triggers when*: The agent adds imports for an uncreated file (e.g., `require('../worker_tenant')`), causing Node.js worker execution to crash instantly.
|
|
|
|
|
- *Target Dimension*: **Narrow Correctness**.
|
|
|
|
|
- *Nearest Acceptable Behavior*: Any helper utility created by the agent is committed as a valid file in the repository or inline logic is used.
|
|
|
|
|
|
|
|
|
|
3. **Unscoped Lookup Security Bypass**:
|
|
|
|
|
- *Triggers when*: An unscoped database query is used to "discover" an owner `userId` before performing authorization checks.
|
|
|
|
|
- *Target Dimension*: **Broader Correctness**.
|
|
|
|
|
- *Nearest Acceptable Behavior*: Trusted `userId` context originates directly from the incoming job message payload.
|
|
|
|
|
|
