Skip to content

fix(experiences): scope idempotency dedup to the author #1803

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ionfwsrijan wants to merge 1 commit into
base: main
Choose a base branch
from
+112 −2
Open

fix(experiences): scope idempotency dedup to the author #1803

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion backend/controllers/interviewExperienceController.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,12 @@ const SECURE_CLIENT_KEY =
const isSecureClientKey = (value) =>
typeof value === "string" && SECURE_CLIENT_KEY.test(value);

// Dedup must be scoped to the author so a re-used idempotencyKey can never
// return another submitter's experience: the submitting user when
// authenticated, otherwise the anonymous clientKey.
const buildAuthorFilter = (req, clientKey) =>
req.user?._id ? { userId: req.user._id } : { clientKey };

const toClientShape = (doc) => {
const obj = typeof doc.toObject === "function" ? doc.toObject() : doc;
return {
Expand Down Expand Up @@ -70,7 +76,10 @@ const createInterviewExperience = async (req, res) => {
payload.color = `hsl(${(payload.company.charCodeAt(0) * 37) % 360}, 55%, 50%)`;
}

const existing = await InterviewExperience.findOne({ idempotencyKey });
const existing = await InterviewExperience.findOne({
idempotencyKey,
...buildAuthorFilter(req, payload.clientKey),
});
if (existing) {
return res.status(200).json({
success: true,
Expand All @@ -92,6 +101,7 @@ const createInterviewExperience = async (req, res) => {
try {
const existing = await InterviewExperience.findOne({
idempotencyKey: req.body.idempotencyKey,
...buildAuthorFilter(req, req.body.clientKey),

@coderabbitai coderabbitai Bot Aug 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Use the normalized idempotency key in duplicate-key recovery.

Line 57 stores the trimmed key, but line 104 queries with the raw request value. If two concurrent requests submit " submit-key-abc12345 ", the losing request cannot find the winning record and returns HTTP 500.

  • backend/controllers/interviewExperienceController.js#L104-L104: retain the validated, trimmed idempotency key and author filter for use in the duplicate-key branch.
  • backend/tests/interviewExperienceController.unit.test.js#L230-L263: add a duplicate-key race test with surrounding whitespace and assert that both lookups use the trimmed key.
🧰 Tools
🪛 ast-grep (0.45.1)

[error] 101-104: Untrusted HTTP request data (req.body / req.query / req.params) flows into a MongoDB/Mongoose query, enabling NoSQL injection — an attacker can supply objects like {"$gt":""} or {"$where":"..."} to bypass filters or run arbitrary JavaScript. Never pass raw request data as a query object or operator value; validate and cast each field to its expected primitive type (e.g. with a schema validator), or whitelist allowed operators before querying.
Context: InterviewExperience.findOne({
idempotencyKey: req.body.idempotencyKey,
...buildAuthorFilter(req, req.body.clientKey),
})
Note: [CWE-943] Improper Neutralization of Special Elements in Data Query Logic.

(nosql-injection-mongo-request-javascript)

📍 Affects 2 files
  • backend/controllers/interviewExperienceController.js#L104-L104 (this comment)
  • backend/tests/interviewExperienceController.unit.test.js#L230-L263
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/controllers/interviewExperienceController.js` at line 104, The
duplicate-key recovery must reuse the validated, trimmed idempotency key and
author filter instead of the raw request value. In
backend/controllers/interviewExperienceController.js:104, retain those
normalized values from the initial validation and pass them to both lookup paths
in the duplicate-key branch. In
backend/tests/interviewExperienceController.unit.test.js:230-263, add a
whitespace-padded duplicate-key race case and assert both lookups use the
trimmed key.

});
if (existing) {
return res.status(200).json({
Expand Down
17 changes: 16 additions & 1 deletion backend/models/InterviewExperience.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,11 +95,26 @@ const interviewExperienceSchema = new mongoose.Schema(
{ timestamps: true },
);

// Idempotency is scoped to the author so a re-used key can never collide
// with another submitter's document: authenticated submissions dedupe by
// userId, anonymous submissions by clientKey.
interviewExperienceSchema.index(
{ idempotencyKey: 1 },
{ userId: 1, idempotencyKey: 1 },
{
unique: true,
partialFilterExpression: {
userId: { $type: "objectId" },
idempotencyKey: { $type: "string", $gt: "" },
},
},
);

interviewExperienceSchema.index(
{ clientKey: 1, idempotencyKey: 1 },
{
unique: true,
partialFilterExpression: {
clientKey: { $type: "string", $gt: "" },
Comment on lines 101 to +117

@coderabbitai coderabbitai Bot Aug 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Make authenticated and anonymous ownership scopes mutually exclusive.

An authenticated request can include a valid clientKey. The controller preserves that value. The document then matches both partial indexes and an anonymous lookup with the same clientKey and idempotencyKey can return the authenticated submission. A second authenticated user can also fail the clientKey unique index despite a different userId.

  • backend/models/InterviewExperience.js#L101-L117: apply the anonymous unique index only to ownerless documents, such as documents where userId is null.
  • backend/controllers/interviewExperienceController.js#L79-L82: remove caller-supplied clientKey for authenticated submissions, remove caller-supplied userId for anonymous submissions, and constrain anonymous lookups to anonymous records.
  • backend/tests/interviewExperienceController.unit.test.js#L207-L228: add coverage for an authenticated submission with clientKey followed by an anonymous submission using that same clientKey and idempotencyKey.
📍 Affects 3 files
  • backend/models/InterviewExperience.js#L101-L117 (this comment)
  • backend/controllers/interviewExperienceController.js#L79-L82
  • backend/tests/interviewExperienceController.unit.test.js#L207-L228
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/models/InterviewExperience.js` around lines 101 - 117, Make
authenticated and anonymous ownership scopes mutually exclusive: in
backend/models/InterviewExperience.js lines 101-117, restrict the anonymous
clientKey/idempotencyKey unique index to documents with userId null; in
backend/controllers/interviewExperienceController.js lines 79-82, strip
caller-supplied clientKey for authenticated requests, strip caller-supplied
userId for anonymous requests, and limit anonymous lookups to ownerless records;
in backend/tests/interviewExperienceController.unit.test.js lines 207-228, add
coverage for an authenticated submission followed by an anonymous submission
reusing the same clientKey and idempotencyKey.

idempotencyKey: { $type: "string", $gt: "" },
},
},
Expand Down
85 changes: 85 additions & 0 deletions backend/tests/interviewExperienceController.unit.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ describe("createInterviewExperience", () => {

expect(InterviewExperience.findOne).toHaveBeenCalledWith({
idempotencyKey: "submit-key-abc12345",
clientKey: "11111111-1111-4111-8111-111111111111",
});
expect(InterviewExperience.create).not.toHaveBeenCalled();
expect(res.status).toHaveBeenCalledWith(200);
Expand All @@ -176,6 +177,90 @@ describe("createInterviewExperience", () => {
}),
);
});

it("scopes the dedup lookup to the authenticated user", async () => {
InterviewExperience.findOne = vi.fn().mockResolvedValue(sampleDoc);
InterviewExperience.create = vi.fn();

const req = makeReq(
{
company: "Google",
role: "SDE-2",
summary: "Tough but fair process",
idempotencyKey: "submit-key-abc12345",
},
{},
{},
{ _id: "507f1f77bcf86cd799439011" },
);
const res = makeRes();

await createInterviewExperience(req, res);

expect(InterviewExperience.findOne).toHaveBeenCalledWith({
idempotencyKey: "submit-key-abc12345",
userId: "507f1f77bcf86cd799439011",
});
expect(res.status).toHaveBeenCalledWith(200);
});

it("lets a different author reuse the same idempotencyKey", async () => {
InterviewExperience.findOne = vi.fn().mockResolvedValue(null);
InterviewExperience.create = vi.fn().mockResolvedValue(sampleDoc);

const req = makeReq({
company: "Google",
role: "SDE-2",
summary: "Tough but fair process",
clientKey: "11111111-1111-4111-8111-111111111111",
idempotencyKey: "submit-key-abc12345",
});
const res = makeRes();

await createInterviewExperience(req, res);

expect(InterviewExperience.findOne).toHaveBeenCalledWith({
idempotencyKey: "submit-key-abc12345",
clientKey: "11111111-1111-4111-8111-111111111111",
});
expect(InterviewExperience.create).toHaveBeenCalled();
expect(res.status).toHaveBeenCalledWith(201);
});

it("scopes the unique-index race lookup to the author", async () => {
InterviewExperience.findOne = vi
.fn()
.mockResolvedValueOnce(null)
.mockResolvedValueOnce(sampleDoc);
InterviewExperience.create = vi
.fn()
.mockRejectedValue({ code: 11000 });

const req = makeReq({
company: "Google",
role: "SDE-2",
summary: "Tough but fair process",
clientKey: "11111111-1111-4111-8111-111111111111",
idempotencyKey: "submit-key-abc12345",
});
const res = makeRes();

await createInterviewExperience(req, res);

expect(InterviewExperience.findOne).toHaveBeenCalledTimes(2);
expect(InterviewExperience.findOne).toHaveBeenNthCalledWith(1, {
idempotencyKey: "submit-key-abc12345",
clientKey: "11111111-1111-4111-8111-111111111111",
});
expect(InterviewExperience.findOne).toHaveBeenNthCalledWith(2, {
idempotencyKey: "submit-key-abc12345",
clientKey: "11111111-1111-4111-8111-111111111111",
});
expect(res.status).toHaveBeenCalledWith(200);
expect(res.json).toHaveBeenCalledWith(
expect.objectContaining({ success: true }),
);
});
});

describe("getMyInterviewExperiences", () => {
Expand Down
Loading