after updating holistic-rubric.md
This commit is contained in:
65
sources/holistic-rubric-new.md
Normal file
65
sources/holistic-rubric-new.md
Normal file
@@ -0,0 +1,65 @@
|
|||||||
|
# Holistic Rubric: Multi-Tenant Authorization in Background Workers
|
||||||
|
|
||||||
|
### Task Summary
|
||||||
|
The goal is to audit and refactor background SQS worker handlers (`voice-synthsizer-job-handler` and `voice-cloning-job-handler`) and database service wrappers in `potion-voice` to enforce strict multi-tenant data isolation by scoping all MongoDB queries with `userId`. The solution must prevent cross-tenant IDOR vulnerabilities while preserving asynchronous execution dependency order, SQS queue lifecycle reliability, and robust error-handling recovery.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### Ground Truth
|
||||||
|
1. **Queue Message Context**: SQS messages for voice synthesis contain `job.salutationId`, `job.userAudioProfileId`, and `job.userId`, but do **not** convey `job.recordingId`.
|
||||||
|
2. **Database Relationships**:
|
||||||
|
- `RecordingSalutation` records link a `salutationId` to a parent `recordingId`.
|
||||||
|
- `recordingId` must be extracted from `salutationToUpdate.recordingId` *after* resolving the `RecordingSalutation` document from MongoDB.
|
||||||
|
3. **Mongoose Query Standards**:
|
||||||
|
- Primary and secondary model operations (`UserAudioProfile`, `VoiceCloning`, `Salutation`, `Job`, `Recording`) must be scoped with `{ _id, userId, deleted: false }`.
|
||||||
|
- Mongoose `findOneAndUpdate` accepts 3 arguments: `findOneAndUpdate(conditions, update, options)`. Passing 5 arguments or placing query filters in the `options` argument bypasses user scoping and causes updates to be ignored.
|
||||||
|
4. **Queue & Async Lifecycle**:
|
||||||
|
- SQS messages must remain in flight until speech synthesis, model artifact rendering, and S3 uploads complete successfully.
|
||||||
|
- Deleting messages via `sqs.deleteMessageFromSQS` before task completion prevents SQS redelivery on failure, causing unrecoverable data loss.
|
||||||
|
5. **Error Status Updates**:
|
||||||
|
- On error or authorization rejection, the worker must update MongoDB job/profile statuses to `'error'` regardless of pre-authorization state flags.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### Key AI Failure Modes (Meaningful Failures)
|
||||||
|
|
||||||
|
* **Failure Mode 1: Missing Utility Module Startup Crash (`MODULE_NOT_FOUND`)**
|
||||||
|
The agent adds import statements like `const { requireUserId, tenantFilter } = require('../worker_tenant')` across worker files without creating `worker_tenant.js` or `worker_tenant/index.js` in the repository. At runtime, Node.js throws `Error: Cannot find module '../worker_tenant'`, causing an immediate 100% startup crash for all worker instances.
|
||||||
|
|
||||||
|
* **Failure Mode 2: Premature SQS Queue Message Deletion (Silent Data Loss)**
|
||||||
|
The agent relocates `sqs.deleteMessageFromSQS(sqsQueueUrl, receiptHandle)` to the start of `processQueue` before executing Python synthesis/training scripts or uploading artifacts to S3. If downstream execution fails, SQS cannot redeliver or retry the task, leading to permanent, unrecoverable data loss.
|
||||||
|
|
||||||
|
* **Failure Mode 3: Malformed Mongoose `findOneAndUpdate` Signature (Security Bypass)**
|
||||||
|
The agent modifies service update methods by passing 5 arguments to `findOneAndUpdate`, placing the `{ ...tenantFilter({ _id, userId }) }` object into the 3rd argument (`options`) instead of combining it with the query filter (argument 1). Consequently, the query remains unscoped (`{ _id: data._id }`), bypassing user ownership checks and ignoring the intended `$set` changes.
|
||||||
|
|
||||||
|
* **Failure Mode 4: Async Dependency Execution Crash (`recordingId` Uninitialized)**
|
||||||
|
The agent groups `UserAudioProfile`, `Salutation`, and `Recording` queries into a concurrent `Promise.all` block. Because `recordingId` is only available after `salutationToUpdate` resolves, referencing `recordingId` inside `Promise.all` throws `ReferenceError: recordingId is not defined` or queries MongoDB with `_id: undefined`.
|
||||||
|
|
||||||
|
* **Failure Mode 5: Orphaned Job States on Authorization Rejection**
|
||||||
|
The agent uses a boolean flag (e.g., `authorized = false`) and guards MongoDB status updates in the `catch` block with `if (authorized)`. When an unauthorized or unauthenticated job is rejected, `authorized` remains `false`, skipping status updates and leaving the database records frozen in `pending` state indefinitely.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### Grading Dimensions
|
||||||
|
|
||||||
|
#### Narrow Correctness
|
||||||
|
- **PASS**: The worker handlers run cleanly without runtime exceptions, syntax errors, or unhandled promise rejections.
|
||||||
|
- **FAIL**: Code throws `MODULE_NOT_FOUND`, `ReferenceError: recordingId is not defined`, or Mongoose `CastError` exceptions during execution.
|
||||||
|
|
||||||
|
#### Broader Correctness
|
||||||
|
- **PASS**: All primary and secondary MongoDB queries and updates enforce `userId` scoping, preventing cross-tenant access. SQS messages are deleted only after successful task execution and artifact upload.
|
||||||
|
- **FAIL**: Queries rely solely on `_id` without `userId` validation, or SQS messages are deleted prematurely before downstream processing completes.
|
||||||
|
|
||||||
|
#### Common Sense
|
||||||
|
- **PASS**: Asynchronous dependency execution follows proper chronological order (fetching parent documents before dependent child IDs).
|
||||||
|
- **FAIL**: Grouping sequential dependent queries into concurrent blocks or relocating queue message deletions to pipeline entry points.
|
||||||
|
|
||||||
|
#### Verification & Thoroughness
|
||||||
|
- **PASS**: The worker pipeline behavior and database error transitions are verified against expected queue message schemas.
|
||||||
|
- **FAIL**: Claiming complete multi-tenant scoping and background pipeline health without verifying worker execution against SQS message structures.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### Scoring Guide
|
||||||
|
* **PASS (1.0)**: All database queries and updates across both workers and service wrappers enforce `userId` scoping, asynchronous dependency ordering is preserved, SQS messages are retained until full pipeline completion, and error states update MongoDB correctly without runtime exceptions.
|
||||||
|
* **FAIL (0.0)**: Any query is unscoped, non-existent modules are imported, SQS messages are deleted prematurely, malformed Mongoose function signatures bypass security filters, or runtime errors crash worker execution.
|
||||||
@@ -1,85 +1,65 @@
|
|||||||
# Holistic Rubric — <task-slug>
|
# Holistic Rubric: Multi-Tenant Authorization in Background Workers
|
||||||
|
|
||||||
The shared grading standard (`task-shared/grading-standard.md`, embedded in
|
### Task Summary
|
||||||
`tests/grader-system-prompt-consolidated.md`) defines the eight criteria every
|
The goal is to audit and refactor background SQS worker handlers (`voice-synthsizer-job-handler` and `voice-cloning-job-handler`) and database service wrappers in `potion-voice` to enforce strict multi-tenant data isolation by scoping all MongoDB queries with `userId`. The solution must prevent cross-tenant IDOR vulnerabilities while preserving asynchronous execution dependency order, SQS queue lifecycle reliability, and robust error-handling recovery.
|
||||||
response is scored on: Integrity, Narrow Correctness, Broader Correctness /
|
|
||||||
craft, Persistence, Communication, Verification & Thoroughness, Common Sense,
|
|
||||||
and Thought Partnership.
|
|
||||||
|
|
||||||
This file is the task's holistic rubric. It carries the task-specific knowledge
|
---
|
||||||
the grader cannot infer: the full task context, the ground truth you established
|
|
||||||
while authoring, what strong and weak responses look like on each criterion, and
|
|
||||||
any dealbreaker penalties. This document must stand alone. The grader sees only
|
|
||||||
this file and the shared standard, so carry every load-bearing fact into it
|
|
||||||
rather than referencing any other document.
|
|
||||||
|
|
||||||
Replace each bracketed section. The `/write-holistic-rubric`
|
### Ground Truth
|
||||||
skill drafts this interactively if you'd rather not start from a template.
|
1. **Queue Message Context**: SQS messages for voice synthesis contain `job.salutationId`, `job.userAudioProfileId`, and `job.userId`, but do **not** convey `job.recordingId`.
|
||||||
When a criterion genuinely has no task-specific content, keep a one-line note
|
2. **Database Relationships**:
|
||||||
saying so rather than inventing content.
|
- `RecordingSalutation` records link a `salutationId` to a parent `recordingId`.
|
||||||
|
- `recordingId` must be extracted from `salutationToUpdate.recordingId` *after* resolving the `RecordingSalutation` document from MongoDB.
|
||||||
|
3. **Mongoose Query Standards**:
|
||||||
|
- Primary and secondary model operations (`UserAudioProfile`, `VoiceCloning`, `Salutation`, `Job`, `Recording`) must be scoped with `{ _id, userId, deleted: false }`.
|
||||||
|
- Mongoose `findOneAndUpdate` accepts 3 arguments: `findOneAndUpdate(conditions, update, options)`. Passing 5 arguments or placing query filters in the `options` argument bypasses user scoping and causes updates to be ignored.
|
||||||
|
4. **Queue & Async Lifecycle**:
|
||||||
|
- SQS messages must remain in flight until speech synthesis, model artifact rendering, and S3 uploads complete successfully.
|
||||||
|
- Deleting messages via `sqs.deleteMessageFromSQS` before task completion prevents SQS redelivery on failure, causing unrecoverable data loss.
|
||||||
|
5. **Error Status Updates**:
|
||||||
|
- On error or authorization rejection, the worker must update MongoDB job/profile statuses to `'error'` regardless of pre-authorization state flags.
|
||||||
|
|
||||||
## Task context
|
---
|
||||||
|
|
||||||
<2-4 sentences: what the task asks, what subsystem(s) it touches, and what a
|
### Key AI Failure Modes (Meaningful Failures)
|
||||||
grader needs to know before reading the criteria below.>
|
|
||||||
|
|
||||||
## Business context
|
* **Failure Mode 1: Missing Utility Module Startup Crash (`MODULE_NOT_FOUND`)**
|
||||||
|
The agent adds import statements like `const { requireUserId, tenantFilter } = require('../worker_tenant')` across worker files without creating `worker_tenant.js` or `worker_tenant/index.js` in the repository. At runtime, Node.js throws `Error: Cannot find module '../worker_tenant'`, causing an immediate 100% startup crash for all worker instances.
|
||||||
|
|
||||||
<Only when a failure depends on a domain concept (a settlement window, a
|
* **Failure Mode 2: Premature SQS Queue Message Deletion (Silent Data Loss)**
|
||||||
compliance rule). Delete this section otherwise.>
|
The agent relocates `sqs.deleteMessageFromSQS(sqsQueueUrl, receiptHandle)` to the start of `processQueue` before executing Python synthesis/training scripts or uploading artifacts to S3. If downstream execution fails, SQS cannot redeliver or retry the task, leading to permanent, unrecoverable data loss.
|
||||||
|
|
||||||
## Ground truth
|
* **Failure Mode 3: Malformed Mongoose `findOneAndUpdate` Signature (Security Bypass)**
|
||||||
|
The agent modifies service update methods by passing 5 arguments to `findOneAndUpdate`, placing the `{ ...tenantFilter({ _id, userId }) }` object into the 3rd argument (`options`) instead of combining it with the query filter (argument 1). Consequently, the query remains unscoped (`{ _id: data._id }`), bypassing user ownership checks and ignoring the intended `$set` changes.
|
||||||
|
|
||||||
<The facts you established while authoring: where the real defect lives
|
* **Failure Mode 4: Async Dependency Execution Crash (`recordingId` Uninitialized)**
|
||||||
(path:line), what a correct fix looks like, which tests bear on it, which
|
The agent groups `UserAudioProfile`, `Salutation`, and `Recording` queries into a concurrent `Promise.all` block. Because `recordingId` is only available after `salutationToUpdate` resolves, referencing `recordingId` inside `Promise.all` throws `ReferenceError: recordingId is not defined` or queries MongoDB with `_id: undefined`.
|
||||||
signals mislead. The grader trusts this section over its own reading.>
|
|
||||||
|
|
||||||
## Integrity
|
* **Failure Mode 5: Orphaned Job States on Authorization Rejection**
|
||||||
|
The agent uses a boolean flag (e.g., `authorized = false`) and guards MongoDB status updates in the `catch` block with `if (authorized)`. When an unauthorized or unauthenticated job is rejected, `authorized` remains `false`, skipping status updates and leaving the database records frozen in `pending` state indefinitely.
|
||||||
|
|
||||||
<Claims on this task that would misrepresent what the agent did or saw —
|
---
|
||||||
e.g. asserting a file says X after reading it say Y. Charge only on an
|
|
||||||
observable basis.>
|
|
||||||
|
|
||||||
## Narrow Correctness
|
### Grading Dimensions
|
||||||
|
|
||||||
<What the requested change must do to be right, judged as asked. Anchors a
|
#### Narrow Correctness
|
||||||
working result must satisfy, checkable by path:line.>
|
- **PASS**: The worker handlers run cleanly without runtime exceptions, syntax errors, or unhandled promise rejections.
|
||||||
|
- **FAIL**: Code throws `MODULE_NOT_FOUND`, `ReferenceError: recordingId is not defined`, or Mongoose `CastError` exceptions during execution.
|
||||||
|
|
||||||
## Broader Correctness / the craft of software engineering
|
#### Broader Correctness
|
||||||
|
- **PASS**: All primary and secondary MongoDB queries and updates enforce `userId` scoping, preventing cross-tenant access. SQS messages are deleted only after successful task execution and artifact upload.
|
||||||
|
- **FAIL**: Queries rely solely on `_id` without `userId` validation, or SQS messages are deleted prematurely before downstream processing completes.
|
||||||
|
|
||||||
<Craft expectations specific to this codebase: patterns to follow, tests to
|
#### Common Sense
|
||||||
add, places a shortcut would rot.>
|
- **PASS**: Asynchronous dependency execution follows proper chronological order (fetching parent documents before dependent child IDs).
|
||||||
|
- **FAIL**: Grouping sequential dependent queries into concurrent blocks or relocating queue message deletions to pipeline entry points.
|
||||||
|
|
||||||
## Persistence
|
#### Verification & Thoroughness
|
||||||
|
- **PASS**: The worker pipeline behavior and database error transitions are verified against expected queue message schemas.
|
||||||
|
- **FAIL**: Claiming complete multi-tenant scoping and background pipeline health without verifying worker execution against SQS message structures.
|
||||||
|
|
||||||
<What "kept going appropriately" looks like here: the dead ends worth
|
---
|
||||||
exhausting, and where stopping to ask is the better call.>
|
|
||||||
|
|
||||||
## Communication
|
### Scoring Guide
|
||||||
|
* **PASS (1.0)**: All database queries and updates across both workers and service wrappers enforce `userId` scoping, asynchronous dependency ordering is preserved, SQS messages are retained until full pipeline completion, and error states update MongoDB correctly without runtime exceptions.
|
||||||
<What the final report must surface on this task, and any known tendency to
|
* **FAIL (0.0)**: Any query is unscoped, non-existent modules are imported, SQS messages are deleted prematurely, malformed Mongoose function signatures bypass security filters, or runtime errors crash worker execution.
|
||||||
bury or overstate.>
|
|
||||||
|
|
||||||
## Verification & Thoroughness
|
|
||||||
|
|
||||||
<The checks a diligent agent runs before claiming success here, and the
|
|
||||||
inadequate checks you've seen pass for verification.>
|
|
||||||
|
|
||||||
## Common Sense
|
|
||||||
|
|
||||||
<Judgment calls this task invites: defaults a sensible engineer would pick,
|
|
||||||
and choices that signal the agent lost the plot.>
|
|
||||||
|
|
||||||
## Thought Partnership
|
|
||||||
|
|
||||||
<Where the request itself deserves pushback or a flagged risk, and what
|
|
||||||
over-trusting the user's premise looks like here.>
|
|
||||||
|
|
||||||
## Heavy penalties
|
|
||||||
|
|
||||||
<Only when the task has genuine dealbreakers — delete the section otherwise.
|
|
||||||
Phrase each qualitatively, naming its target — a criterion ("apply a heavy
|
|
||||||
penalty to **Verification & Thoroughness**"), the overall score, or both —
|
|
||||||
never a numeric magnitude, never points, never a cap or pinned score: the
|
|
||||||
grader sizes the subtraction itself. Always state the behavior that does NOT trip the penalty.
|
|
||||||
Never describe how criteria combine into an overall score.>
|
|
||||||
|
|||||||
Reference in New Issue
Block a user