Skip to content

fix: enforce catalog integrity for order item creation #59

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
dharapandya85 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: enforce catalog integrity for order item creation #59

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
3 changes: 3 additions & 0 deletions backend/db.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,9 @@ export const database = {

createOrder({ spotId, userId, items }) {
const parsedItems = items.map((item) => {
if ('name' in item || 'unitPrice' in item || 'total' in item) {
throw new Error('Do not provide name, unitPrice, or total. These are derived from catalog. ')
}
Comment on lines 244 to +247
Copy link

@coderabbitai coderabbitai bot Mar 4, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

JavaScript in operator behavior with null undefined primitives TypeError

💡 Result:

In JavaScript, the in operator requires its right-hand operand to be an Object. Per the ECMAScript spec, evaluating prop in rhs does:

  • evaluate rhs to a value rVal
  • if rVal is not an Object, throw TypeError
  • otherwise return HasProperty(rVal, ToPropertyKey(prop)) (checks own + prototype chain) [1]

So these all throw, because the RHS is not an Object (including null, undefined, and all primitives like strings/numbers/booleans/bigints/symbols):

"x" in null        // TypeError
"x" in undefined   // TypeError
"length" in "abc"  // TypeError

This is the same error described by MDN (“right-hand side of 'in' should be an object”) and it explicitly calls out null/undefined and primitives as causes [2].

How to avoid the TypeError

1) Guard against nullish values (most common):

if (obj != null && ("x" in obj)) { ... }  // obj is not null/undefined

2) If you intentionally want “wrap primitives” behavior:

"x" in Object(maybeValue)   // never throws for null/undefined; primitives get boxed

(Be aware this changes semantics vs throwing—for null/undefined it becomes {}, for strings it becomes a String object wrapper.)

3) If you just want a safe property read (not in):
Use optional chaining for access:

const v = obj?.x; // undefined if obj is null/undefined

[3]

Sources: ECMAScript spec (RelationalExpression in) [1], MDN error reference for in with non-objects [2], MDN optional chaining [3].

🌐 Web query:

JavaScript in operator prototype chain inheritance check

💡 Result:

In JavaScript, the in operator returns true if a property exists either as an own property or anywhere on the object’s prototype chain:

const proto = { x: 1 };
const obj = Object.create(proto);

"x" in obj;          // true  (found on prototype)
Object.hasOwn(obj,"x"); // false (not an own property)

Key points:

  • prop in obj checks existence, not value—so it can be true even if obj[prop] === undefined. [1]
  • It includes inherited properties from prototypes, which is why it’s often not the right choice for “own property only” checks; use Object.hasOwn(obj, prop) (or hasOwnProperty.call) for that. [1]
  • If what you really want is an inheritance/prototype-chain relationship check (not a property check), use:
    • someProto.isPrototypeOf(obj) to test whether someProto appears in obj’s prototype chain, or
    • obj instanceof Ctor (which checks against Ctor.prototype). [2]

Sources:
[1] MDN — in operator (prototype chain behavior, Object.hasOwn recommendation) (developer.mozilla.org)
[2] MDN — Object.prototype.isPrototypeOf() (prototype chain relationship; note about instanceof) (developer.mozilla.org)

Citations:

🏁 Script executed:

# Find and examine the backend/db.js file around the changed lines
find . -name "db.js" -path "*/backend/*" | head -5

Repository: fuzziecoder/Brocode-Party-Update-App

Length of output: 95

🏁 Script executed:

# Read the broader context around lines 244-247 in backend/db.js
fd "db.js" backend | xargs wc -l

Repository: fuzziecoder/Brocode-Party-Update-App

Length of output: 97

🏁 Script executed:

# Read the backend/db.js file with line numbers, focusing on the function context around lines 244-247
sed -n '230,260p' backend/db.js | cat -n

Repository: fuzziecoder/Brocode-Party-Update-App

Length of output: 1364

🏁 Script executed:

# Read more context above createOrder to see if there's any upstream validation
sed -n '200,260p' backend/db.js | cat -n

Repository: fuzziecoder/Brocode-Party-Update-App

Length of output: 2620

🏁 Script executed:

# Check the broader function to see if items is validated anywhere before createOrder is called
sed -n '1,100p' backend/db.js | grep -A5 -B5 "createOrder\|items" | head -30

Repository: fuzziecoder/Brocode-Party-Update-App

Length of output: 997

Add type guard before using in operator on item; can throw TypeError on malformed payloads.

At line 246, the in operator throws a TypeError if item is null, undefined, or a primitive value. This can occur with malformed client input and will cause an unhandled runtime error instead of a proper validation error. Additionally, in checks the prototype chain; use Object.prototype.hasOwnProperty.call() to validate only own properties.

Suggested fix 
   const parsedItems = items.map((item) => {
+    if (!item || typeof item !== 'object' || Array.isArray(item)) {
+      throw new Error('Each order item must be an object with productId and a positive integer quantity');
+    }
-    if ('name' in item || 'unitPrice' in item || 'total' in item) {
-      throw new Error('Do not provide name, unitPrice, or total. These are derived from catalog. ')
-    }
+    if (
+      Object.prototype.hasOwnProperty.call(item, 'name') ||
+      Object.prototype.hasOwnProperty.call(item, 'unitPrice') ||
+      Object.prototype.hasOwnProperty.call(item, 'total')
+    ) {
+      throw new Error('Do not provide name, unitPrice, or total. These are derived from catalog.');
+    }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const parsedItems = items.map((item) => {
if ('name' in item || 'unitPrice' in item || 'total' in item) {
throw new Error('Do not provide name, unitPrice, or total. These are derived from catalog. ')
}
const parsedItems = items.map((item) => {
if (!item || typeof item !== 'object' || Array.isArray(item)) {
throw new Error('Each order item must be an object with productId and a positive integer quantity');
}
if (
Object.prototype.hasOwnProperty.call(item, 'name') ||
Object.prototype.hasOwnProperty.call(item, 'unitPrice') ||
Object.prototype.hasOwnProperty.call(item, 'total')
) {
throw new Error('Do not provide name, unitPrice, or total. These are derived from catalog.');
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@backend/db.js` around lines 244 - 247, The map callback used to build
parsedItems currently uses the 'in' operator on item which can throw a TypeError
for null/undefined/primitives and also checks prototype properties; before
accessing 'in' replace that with a type guard that ensures item is a non-null
object (e.g., typeof item === 'object' && item !== null) and then use
Object.prototype.hasOwnProperty.call(item, 'name') / 'unitPrice' / 'total' to
check only own properties; update the items.map callback to validate and throw
the existing Error when those own properties are present, and otherwise continue
parsing so malformed payloads produce a controlled validation error instead of a
runtime TypeError.

const quantity = Number(item.quantity || 0);
if (!item.productId || !Number.isInteger(quantity) || quantity <= 0) {
throw new Error('Each order item must include productId and a positive integer quantity');
Expand Down