-
Notifications
You must be signed in to change notification settings - Fork 0
Cursor new oma req review #57
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -11,6 +11,13 @@ router.get('/test', (req, res) => { | |||||||||||||||||||||||||||||||||||||
| res.status(200).json({ message: 'Test route is working' }); | ||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| // Smoke-only: string concat instead of numeric add (wrong for "1"+"2" expectation). | ||||||||||||||||||||||||||||||||||||||
| router.get('/cr-smoke-sum', (req, res) => { | ||||||||||||||||||||||||||||||||||||||
| const a = req.query.a; | ||||||||||||||||||||||||||||||||||||||
| const b = req.query.b; | ||||||||||||||||||||||||||||||||||||||
| res.json({ sum: a + b }); | ||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+15
to
+19
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Parse and validate the operands before returning This endpoint currently returns the wrong result for numeric inputs: Suggested fix router.get('/cr-smoke-sum', (req, res) => {
- const a = req.query.a;
- const b = req.query.b;
- res.json({ sum: a + b });
+ const a = Number(req.query.a);
+ const b = Number(req.query.b);
+
+ if (!Number.isFinite(a) || !Number.isFinite(b)) {
+ return res.status(400).json({
+ success: false,
+ message: 'Query params a and b must be numbers',
+ });
+ }
+
+ res.json({ sum: a + b });
});📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| router.use('/webhooks', webhookRoutes); | ||||||||||||||||||||||||||||||||||||||
| router.use('/orders', orderRoutes); | ||||||||||||||||||||||||||||||||||||||
| router.use('/printers', printerRoutes); | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove the hardcoded token and stop authenticating via
req.query.This adds a credential to source control and accepts it from the URL, which is easy to leak through logs, browser history, and proxies. If this route must exist, load the token through
backend/src/config/env.jsand read it from a header instead; otherwise remove or non-prod-gate the endpoint.As per coding guidelines,
backend/src/**/*.{js,ts}: Backend source code must not contain hardcoded credentials, Shopify webhook secrets, or database passwords (must use env variables andconfig/env.jspatterns).🤖 Prompt for AI Agents