Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 7 additions & 3 deletions backend/controllers/data.controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ module.exports.insertData = async (req, res) => {
}
}

// Prevnt NoSQL injection
const safeData = sanitize(cleanData);

let docSize = 0;
Expand All @@ -66,6 +65,7 @@ module.exports.insertData = async (req, res) => {
console.timeEnd("insert data")
res.status(201).json(result);
} catch (err) {
console.error(err);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

While console.error is useful, for production environments it's recommended to use a structured logger (e.g., Winston, Pino). Structured logging provides more context with log levels, can output in machine-readable formats like JSON for easier analysis with log management tools, and offers more flexible configuration.

res.status(500).json({ error: err.message });
}
};
Expand All @@ -92,6 +92,7 @@ module.exports.getAllData = async (req, res) => {
console.timeEnd("getall")
res.json(data);
} catch (err) {
console.error(err);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

security-high high

The getAllData function is vulnerable to NoSQL injection because it passes the unsanitized req.query object to the QueryEngine. The QueryEngine.filter method directly assigns query parameters to the MongoDB query object, allowing an attacker to use MongoDB operators (e.g., ?field[$ne]=value) to bypass filters or extract sensitive data. Although this line adds error logging, the underlying function remains insecure.

res.status(500).json({ error: err.message });
}
};
Expand All @@ -116,6 +117,7 @@ module.exports.getSingleDoc = async (req, res) => {

res.json(doc);
} catch (err) {
console.error(err);
res.status(500).json({ error: err.message });
}
};
Expand All @@ -135,12 +137,12 @@ module.exports.updateSingleData = async (req, res) => {
const connection = await getConnection(project._id);
const Model = getCompiledModel(connection, collectionConfig, project._id, project.resources.db.isExternal);

// Strict Schema Validation
// Schma Validation

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

There's a typo in this comment. It should be 'Schema'.

Suggested change
// Schma Validation
// Schema Validation

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Fix typo in validation comment.

Line 140 says Schma Validation; please rename it to Schema Validation for clarity.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@backend/controllers/data.controller.js` at line 140, Replace the typo in the
inline comment reading "Schma Validation" with the correct "Schema Validation"
so the validation section comment is clear and accurate (search for the exact
comment string "Schma Validation" in backend/controllers/data.controller.js and
update it to "Schema Validation").

const schemaRules = collectionConfig.model;
const updateData = {};
for (const key in incomingData) {
const fieldRule = schemaRules.find(f => f.key === key);
if (!fieldRule) continue; // Unknown fields ko ignore karo
if (!fieldRule) continue;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

security-high high

The updateSingleData function is vulnerable to NoSQL injection, particularly for fields defined as 'Date' in the schema. The validation loop modified here lacks a type check for 'Date' fields, allowing an attacker to provide an object containing MongoDB operators (e.g., {"myDateField": {"$gt": ""}}). Since the sanitize function only checks top-level keys for the $ prefix, these nested operators will bypass sanitization and be passed directly to Model.findByIdAndUpdate, potentially leading to unauthorized data manipulation.


const value = incomingData[key];
if (fieldRule.type === 'Number' && typeof value !== 'number') return res.status(400).json({ error: `Field '${key}' must be a Number.` });
Expand All @@ -157,6 +159,7 @@ module.exports.updateSingleData = async (req, res) => {

res.json({ message: "Updated", data: result });
} catch (err) {
console.error(err);
res.status(500).json({ error: err.message });
}
};
Expand Down Expand Up @@ -195,6 +198,7 @@ module.exports.deleteSingleDoc = async (req, res) => {

res.json({ message: "Document deleted", id });
} catch (err) {
console.error(err);
res.status(500).json({ error: err.message });
}
};
11 changes: 4 additions & 7 deletions backend/controllers/schema.controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,17 +19,16 @@ module.exports.checkSchema = async (req, res) => {

res.status(200).json({ message: "Schema exists", collection: collectionConfig });
} catch (err) {
console.error(err);
res.status(500).json({ error: err.message });
}
};

module.exports.createSchema = async (req, res) => {
try {
const { name, fields } = createSchemaApiKeySchema.parse(req.body);
let project = req.project; // From verifyApiKey
let project = req.project;

// Need the full document to call .save(), req.project from cache might be a lean object.
// It's safer to fetch the mongoose model instance to modify it.
const projectId = project._id;
const fullProject = await Project.findById(projectId);

Expand All @@ -40,9 +39,7 @@ module.exports.createSchema = async (req, res) => {

if (!fullProject.jwtSecret) fullProject.jwtSecret = uuidv4();

// Convert the incoming API payload (name, type, required) into the UrBackend schema format (key, type, required)
const transformedFields = (fields || []).map(f => {
// Capitalize first letter of type mapping "string" -> "String" requirement
const mappedType = f.type.charAt(0).toUpperCase() + f.type.slice(1).toLowerCase();
return {
key: f.name,
Expand All @@ -57,8 +54,7 @@ module.exports.createSchema = async (req, res) => {
// Clear redis cache
await deleteProjectById(projectId.toString());
await setProjectById(projectId.toString(), fullProject);
await deleteProjectByApiKeyCache(fullProject.apiKey); // In case it was cached by hashed key
// NOTE: req.hashedApiKey is available from middleware if we strictly want that.
await deleteProjectByApiKeyCache(fullProject.apiKey);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Check Project model for apiKey requirement/default =="
fd 'Project\.(js|ts)$' | while read -r f; do
  echo "--- $f ---"
  rg -n -C3 'apiKey|required|default' "$f" || true
done

echo
echo "== Inspect deleteProjectByApiKeyCache null/falsy handling =="
fd 'redisCaching\.(js|ts)$' | while read -r f; do
  echo "--- $f ---"
  rg -n -C4 'deleteProjectByApiKeyCache|apiKey|toString\(' "$f" || true
done

Repository: yash-pouranik/urBackend

Length of output: 2373


🏁 Script executed:

# Find schema.controller.js and read context around line 57
fd 'schema\.controller\.js$' | head -1 | xargs cat -n | sed -n '40,75p'

Repository: yash-pouranik/urBackend

Length of output: 1535


Add a guard before invalidating API-key cache.

Line 57 passes fullProject.apiKey directly to deleteProjectByApiKeyCache, which lacks null-safety and would create a malformed cache key (e.g., project:apikey:null) if the value is ever falsy. Though the schema requires apiKey, defensive guarding aligns with the pattern already used at lines 58–60 for req.hashedApiKey.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@backend/controllers/schema.controller.js` at line 57, Add a null-safety guard
before calling deleteProjectByApiKeyCache: check that fullProject &&
fullProject.apiKey is truthy and only then call
deleteProjectByApiKeyCache(fullProject.apiKey). Mirror the existing pattern used
for req.hashedApiKey (lines handling req.hashedApiKey) so you avoid constructing
malformed cache keys when fullProject.apiKey is falsy.

if (req.hashedApiKey) {
await deleteProjectByApiKeyCache(req.hashedApiKey);
}
Expand All @@ -70,6 +66,7 @@ module.exports.createSchema = async (req, res) => {
res.status(201).json({ message: "Schema created successfully", project: projectObj });
} catch (err) {
if (err instanceof z.ZodError) return res.status(400).json({ error: err.errors });
console.error(err);
res.status(500).json({ error: err.message });
}
};