Skip to content

test: custom bot_name=code-bot #14

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

test: custom bot_name=code-bot #14

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
20 changes: 20 additions & 0 deletions .github/workflows/ai-review-custom-bot.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
name: AI Code Review (CodeBot)
on:
pull_request:
types: [opened, synchronize, reopened]

jobs:
review:
runs-on: ubuntu-latest
permissions:
contents: read
pull-requests: write
steps:
- uses: actions/checkout@v4
with:
fetch-depth: 0
- uses: concretios/ai-pr-reviewer@v1
with:
gemini_api_key: ${{ secrets.GEMINI_API_KEY }}
bot_name: 'code-bot'
submit_review_verdict: true
2 changes: 2 additions & 0 deletions app.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
const express = require('express');
const tasksRouter = require('./routes/tasks');
const tagsRouter = require('./routes/tags');

const app = express();
const PORT = process.env.PORT || 3000;
Expand All @@ -11,6 +12,7 @@ app.get('/health', (req, res) => {
});

app.use('/tasks', tasksRouter);
app.use('/tags', tagsRouter);

app.listen(PORT, () => {
console.log(`Server running on port ${PORT}`);
Expand Down
53 changes: 53 additions & 0 deletions routes/tags.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
const express = require('express');
const router = express.Router();

// In-memory tag store
let tags = [];
let nextId = 1;

@github-actions github-actions Bot Apr 8, 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.

🩺 [MEDIUM] architecture: Missing Pagination for GET /tags Endpoint

The /tags list endpoint does not implement pagination (page and limit query parameters) as required by the API design rules. This can lead to performance issues and large data transfers for a growing number of tags.

Suggestion:

Suggested change
router.get('/', (req, res) => {
const page = parseInt(req.query.page) || 1;
const limit = parseInt(req.query.limit) || 10;
const startIndex = (page - 1) * limit;
const endIndex = page * limit;
const paginatedTags = tags.slice(startIndex, endIndex);
res.json({ data: paginatedTags, meta: { page, limit, total: tags.length } });
});

// GET /tags
router.get('/', (req, res) => {
res.json(tags);

@github-actions github-actions Bot Apr 8, 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.

🩺 [LOW] style: Missing JSDoc Comments for Route Handlers

None of the new route handlers have JSDoc comments describing their parameters or return values. This violates the coding style rule that 'All functions must have JSDoc comments.'

@github-actions github-actions Bot Apr 8, 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.

🩺 [LOW] quality: Missing Error Handling (Try/Catch)

None of the route handlers use try/catch blocks or leverage Express error middleware for explicit error handling. While Express handles basic synchronous errors, this can lead to unhandled promise rejections or inconsistent error responses for more complex logic. This violates the rule 'All route handlers must use try/catch or express error middleware.'

});

@github-actions github-actions Bot Apr 8, 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.

🩺 [MEDIUM] architecture: Missing Pagination for List Endpoint

The GET /tags endpoint does not implement pagination using page and limit query parameters. This violates the API design rule that 'All list endpoints must support pagination,' which can lead to performance issues with large datasets.

@github-actions github-actions Bot Apr 8, 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.

🩺 [MEDIUM] architecture: Incorrect List Response Format

The GET /tags endpoint returns the raw tags array directly. This violates the API design rule that 'Response format for lists: { data: [], meta: { page, limit, total } },' leading to an inconsistent API response structure.


// GET /tags/:id
router.get('/:id', (req, res) => {

@github-actions github-actions Bot Apr 8, 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.

🩺 [MEDIUM] correctness: Loose Equality (==) for ID Comparison

Using == for comparing t.id (number) and req.params.id (string) can lead to unexpected type coercion. It's safer and more explicit to parse req.params.id to a number and use strict equality ===.

Suggestion:

Suggested change
router.get('/:id', (req, res) => {
const tag = tags.find(t => t.id === parseInt(req.params.id, 10));

const tag = tags.find(t => t.id == req.params.id);

@github-actions github-actions Bot Apr 8, 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.

🩺 [MEDIUM] quality: Inconsistent Error Response Format

The error response { error: 'Tag not found' } is missing the code field, which is required by the API design patterns for all error responses. This inconsistency makes error handling more difficult for clients.

Suggestion:

Suggested change
const tag = tags.find(t => t.id == req.params.id);
if (!tag) return res.status(404).json({ error: 'Tag not found', code: 'TAG_NOT_FOUND' });

if (!tag) return res.status(404).json({ error: 'Tag not found' });

@github-actions github-actions Bot Apr 8, 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.

🩺 [HIGH] correctness: Type Coercion in ID Comparison

The id parameter from req.params is compared using == instead of ===. This can lead to unexpected behavior due to JavaScript's type coercion rules, potentially matching incorrect IDs if types differ. This issue is present in GET, PATCH, and DELETE routes.

Suggestion:

Suggested change
if (!tag) return res.status(404).json({ error: 'Tag not found' });
const tag = tags.find(t => t.id === parseInt(req.params.id, 10));

res.json(tag);
});

// POST /tags
router.post('/', (req, res) => {
const { name, color } = req.body;

@github-actions github-actions Bot Apr 8, 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.

🩺 [CRITICAL] security: Missing Authentication on Mutating Endpoints

The POST, PATCH, and DELETE endpoints for /tags are not protected by any authentication middleware. This allows any unauthenticated user to create, modify, or delete tags, which is a critical security vulnerability. All API endpoints that modify data must require authentication.

if (!name) {

@github-actions github-actions Bot Apr 8, 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.

🩺 [HIGH] security: Inadequate Input Validation and Sanitization

User input for name and color in POST and PATCH requests is not properly validated or sanitized. The POST endpoint only checks for the presence of name, and color is not validated at all. This violates the rule to 'Validate and sanitize all user input at the route handler level' and can lead to incorrect data or potential injection issues.

return res.status(400).json({ error: 'Name is required' });

@github-actions github-actions Bot Apr 8, 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.

🩺 [MEDIUM] architecture: Inline Request Validation

The validation for name in the POST /tags endpoint is performed inline within the route handler. This violates the 'Use middleware for request validation, not inline checks in route handlers' rule, leading to less modular and reusable code.

}

@github-actions github-actions Bot Apr 8, 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.

🩺 [CRITICAL] security: Missing Authentication on POST /tags Endpoint

The POST /tags endpoint, which creates new resources, does not have any authentication middleware applied. This allows any unauthenticated user to create tags, violating the security rules requiring authentication for all mutating endpoints.

Suggestion:

Suggested change
}
router.post('/', authenticate, (req, res) => {

@github-actions github-actions Bot Apr 8, 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.

🩺 [HIGH] security: Missing Input Validation and Sanitization for POST /tags

The POST /tags endpoint only checks for the presence of name but performs no validation or sanitization on name or color. This can lead to injection attacks (e.g., XSS if name is later rendered in HTML) or incorrect data being stored. All user input must be validated and sanitized.

Suggestion:

Suggested change
}
// Add a validation middleware here, e.g., using Joi or express-validator
// Example: validateTagCreation, (req, res) => {

@github-actions github-actions Bot Apr 8, 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.

🩺 [MEDIUM] quality: Inconsistent Error Response Format

Error responses like res.status(400).json({ error: 'Name is required' }) are missing the code field. This violates the rule for a consistent error response format: { error: string, code?: string }. This issue is present in GET, POST, and DELETE routes.

Suggestion:

Suggested change
}
return res.status(400).json({ error: 'Name is required', code: 'NAME_REQUIRED' });

const tag = {

@github-actions github-actions Bot Apr 8, 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.

🩺 [MEDIUM] style: Inline Validation Violates API Design Pattern

The if (!name) check is an inline validation. The API design patterns explicitly state to use middleware for request validation, not inline checks in route handlers. This improves separation of concerns and reusability.

Suggestion:

Suggested change
const tag = {
// Move validation to a dedicated middleware function.

id: nextId++,

@github-actions github-actions Bot Apr 8, 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.

🩺 [MEDIUM] quality: Inconsistent Error Response Format for POST /tags

The error response for missing name in POST /tags is missing the code field, violating the consistent error response format rule. Include a machine-readable code for better client-side error handling.

Suggestion:

Suggested change
id: nextId++,
return res.status(400).json({ error: 'Name is required', code: 'NAME_REQUIRED' });

name: name,
color: color || '#000000',
createdAt: new Date().toISOString()
};
tags.push(tag);
res.status(201).json(tag);
});

@github-actions github-actions Bot Apr 8, 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.

🩺 [MEDIUM] quality: Missing 'updatedAt' Timestamp on Tag Resources

The createdAt timestamp is added, but the updatedAt timestamp is missing from the tag resource. The coding standards require both createdAt and updatedAt on all resources for better data traceability.

Suggestion:

Suggested change
});
createdAt: new Date().toISOString(),
updatedAt: new Date().toISOString()


// PATCH /tags/:id
router.patch('/:id', (req, res) => {
const tag = tags.find(t => t.id == req.params.id);
if (!tag) return res.status(404).json({ error: 'Tag not found' });
if (req.body.name) tag.name = req.body.name;
if (req.body.color) tag.color = req.body.color;
res.json(tag);

@github-actions github-actions Bot Apr 8, 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.

🩺 [CRITICAL] security: Missing Authentication on PATCH /tags/:id Endpoint

The PATCH /tags/:id endpoint, which modifies existing resources, lacks authentication. This allows any unauthenticated user to update tags, which is a critical security vulnerability.

Suggestion:

Suggested change
res.json(tag);
router.patch('/:id', authenticate, (req, res) => {

@github-actions github-actions Bot Apr 8, 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.

🩺 [HIGH] security: Missing Input Validation and Sanitization for PATCH /tags/:id

The PATCH /tags/:id endpoint does not validate or sanitize req.params.id, req.body.name, or req.body.color. This opens the door to potential vulnerabilities like type coercion issues with id and data integrity problems with name and color.

Suggestion:

Suggested change
res.json(tag);
// Add a validation middleware here, e.g., using Joi or express-validator
// Example: validateTagUpdate, (req, res) => {

@github-actions github-actions Bot Apr 8, 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.

🩺 [MEDIUM] correctness: Loose Equality (==) for ID Comparison in PATCH

Similar to the GET endpoint, using == for ID comparison in the PATCH route can lead to type coercion issues. Always use strict equality after parsing the parameter.

Suggestion:

Suggested change
res.json(tag);
const tag = tags.find(t => t.id === parseInt(req.params.id, 10));

});

@github-actions github-actions Bot Apr 8, 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.

🩺 [MEDIUM] architecture: Missing updatedAt Timestamp on Update

The PATCH /tags/:id endpoint updates name and color but does not include an updatedAt timestamp. This violates the API design rule to 'Include createdAt and updatedAt timestamps on all resources.'

Suggestion:

Suggested change
});
if (req.body.name) tag.name = req.body.name;
if (req.body.color) tag.color = req.body.color;
tag.updatedAt = new Date().toISOString();
res.json(tag);


// DELETE /tags/:id
router.delete('/:id', (req, res) => {

@github-actions github-actions Bot Apr 8, 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.

🩺 [MEDIUM] quality: Missing 'updatedAt' Update on PATCH /tags/:id

When a tag is updated via PATCH, the updatedAt timestamp is not being updated. This violates the rule to include and maintain updatedAt timestamps on all resources.

Suggestion:

Suggested change
router.delete('/:id', (req, res) => {
if (req.body.color) tag.color = req.body.color;
tag.updatedAt = new Date().toISOString();

const idx = tags.findIndex(t => t.id == req.params.id);
if (idx === -1) return res.status(404).json({ error: 'Tag not found' });
tags.splice(idx, 1);
res.status(204).send();

@github-actions github-actions Bot Apr 8, 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.

🩺 [CRITICAL] security: Missing Authentication on DELETE /tags/:id Endpoint

The DELETE /tags/:id endpoint, which removes resources, is not protected by authentication. This allows any unauthenticated user to delete tags, posing a significant security risk.

Suggestion:

Suggested change
res.status(204).send();
router.delete('/:id', authenticate, (req, res) => {

@github-actions github-actions Bot Apr 8, 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.

🩺 [MEDIUM] correctness: Loose Equality (==) for ID Comparison in DELETE

The DELETE endpoint also uses == for ID comparison, which is prone to type coercion. Convert req.params.id to an integer and use === for reliable matching.

Suggestion:

Suggested change
res.status(204).send();
const idx = tags.findIndex(t => t.id === parseInt(req.params.id, 10));

});

@github-actions github-actions Bot Apr 8, 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.

🩺 [MEDIUM] quality: Inconsistent Error Response Format for DELETE /tags/:id

The error response for a non-existent tag in DELETE /tags/:id is missing the code field. All error responses should include a machine-readable code for consistency.

Suggestion:

Suggested change
});
if (idx === -1) return res.status(404).json({ error: 'Tag not found', code: 'TAG_NOT_FOUND' });


module.exports = router;
Loading