[1522] Proj_Router Logging Improvements #1822
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
megatnt1122
added
Type: Refactor
Contributor
Reviewer's GuideAdds structured request logging to all proj_router endpoints and introduces a comprehensive Foxx unit test suite for the project router, wired into the CMake Foxx test harness. Sequence diagram for proj_router GET /create with structured loggingsequenceDiagram
actor Client
participant ProjRouter as proj_router
participant Logger as logger
participant DB as g_db
participant Lib as g_lib
Client->>ProjRouter: HTTP GET /prj/create?client=...
ProjRouter->>Logger: logRequestStarted(client, correlationId, httpVerb, routePath, status, description)
ProjRouter->>DB: _executeTransaction({...}, action)
activate DB
DB-->>ProjRouter: result
deactivate DB
ProjRouter->>Client: res.send(result)
ProjRouter->>Logger: logRequestSuccess(client, correlationId, httpVerb, routePath, status, description, extra)
alt error during processing
ProjRouter->>Logger: logRequestFailure(client, correlationId, httpVerb, routePath, status, description, extra, error)
ProjRouter->>Lib: handleException(error, res)
Lib-->>Client: Error response
end
Sequence diagram for proj_router GET /view with logging and permission checkssequenceDiagram
actor Client
participant ProjRouter as proj_router
participant Logger as logger
participant Lib as g_lib
participant PCollection as g_db_p
Client->>ProjRouter: HTTP GET /prj/view?id=...&client=...
ProjRouter->>Logger: logRequestStarted(client, correlationId, GET, prj/view, Started, description)
ProjRouter->>Lib: getUserFromClientID_noexcept(client)
Lib-->>ProjRouter: clientUser
ProjRouter->>PCollection: exists(projectId)
PCollection-->>ProjRouter: existsFlag
ProjRouter->>PCollection: document({_id: projectId})
PCollection-->>ProjRouter: proj
ProjRouter->>Client: res.send([proj])
ProjRouter->>Logger: logRequestSuccess(client, correlationId, GET, prj/view, Success, description, result)
alt error during view
ProjRouter->>Logger: logRequestFailure(client, correlationId, GET, prj/view, Failure, description, extra, error)
ProjRouter->>Lib: handleException(error, res)
Lib-->>Client: Error response
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 3 security issues, 6 other issues, and left some high level feedback:
Security issues:
- Detected user input used to manually construct a SQL string. This is usually bad practice because manual construction could accidentally result in a SQL injection. An attacker could use a SQL injection to steal or modify contents of the database. Instead, use a parameterized query which is available by default in most database engines. Alternatively, consider using an object-relational mapper (ORM) such as Sequelize which will protect your queries. (link)
- Detected user input used to manually construct a SQL string. This is usually bad practice because manual construction could accidentally result in a SQL injection. An attacker could use a SQL injection to steal or modify contents of the database. Instead, use a parameterized query which is available by default in most database engines. Alternatively, consider using an object-relational mapper (ORM) such as Sequelize which will protect your queries. (link)
- Detected user input used to manually construct a SQL string. This is usually bad practice because manual construction could accidentally result in a SQL injection. An attacker could use a SQL injection to steal or modify contents of the database. Instead, use a parameterized query which is available by default in most database engines. Alternatively, consider using an object-relational mapper (ORM) such as Sequelize which will protect your queries. (link)
General comments:
- In the
/get_roleroute’s catch block you are callinglogger.logRequestSuccessinstead of a failure variant, which will mislabel error cases in the logs; this should be changed to the appropriate failure logging call. - The log messages for
/get_roleuseidsin the description string while the query parameter is namedid, which could be confusing when correlating logs with requests; align the wording with the actual parameter name. - For the
/searchroute, the failure logger currently omits the error object (unlike other routes whereerror: eis passed); consider including the error there as well for consistency and easier troubleshooting.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In the `/get_role` route’s catch block you are calling `logger.logRequestSuccess` instead of a failure variant, which will mislabel error cases in the logs; this should be changed to the appropriate failure logging call.
- The log messages for `/get_role` use `ids` in the description string while the query parameter is named `id`, which could be confusing when correlating logs with requests; align the wording with the actual parameter name.
- For the `/search` route, the failure logger currently omits the error object (unlike other routes where `error: e` is passed); consider including the error there as well for consistency and easier troubleshooting.
## Individual Comments
### Comment 1
<location> `core/database/foxx/api/proj_router.js:804-807` </location>
<code_context>
+ httpVerb: "GET",
+ routePath: basePath + "/get_role",
+ status: "Started",
+ description: `Get client/subject project role. ID:${req.queryParams.ids}`,
+ });
+
</code_context>
<issue_to_address>
**suggestion (bug_risk):** `description` string uses `ids` while the rest of the handler uses `id`.
This uses `req.queryParams.ids`, but elsewhere in `/get_role` the query parameter is `id`. If the param is actually singular, this will log `undefined` and make debugging harder. Please update the description to use the correct property name.
```suggestion
httpVerb: "GET",
routePath: basePath + "/get_role",
status: "Started",
description: `Get client/subject project role. ID:${req.queryParams.id}`,
```
</issue_to_address>
### Comment 2
<location> `core/database/foxx/api/proj_router.js:704` </location>
<code_context>
+ httpVerb: "GET",
+ routePath: basePath + "/search",
+ status: "Started",
+ description: `Find all projects that match query: ${req.queryParams.query}`,
+ });
+
</code_context>
<issue_to_address>
**🚨 suggestion (security):** Logging full query text may expose sensitive data or produce very large log entries.
The start/success/failure logs for `/search` interpolate `req.queryParams.query` directly into the message. Since the AQL string may include user data and can be large, consider truncating, sanitizing, or hashing it before logging to reduce sensitive-data exposure and control log size.
Suggested implementation:
```javascript
const rawQuery = typeof req.queryParams.query === "string" ? req.queryParams.query : "";
const safeQuerySnippet =
rawQuery.length > 200 ? rawQuery.slice(0, 200) + "…[truncated]" : rawQuery;
logger.logRequestStarted({
client: req.queryParams.client,
correlationId: req.headers["x-correlation-id"],
httpVerb: "GET",
routePath: basePath + "/search",
status: "Started",
description: `Find all projects that match query (preview): ${safeQuerySnippet}`,
});
```
You mentioned similar start/success/failure logs for `/search`. Apply the same `rawQuery`/`safeQuerySnippet` pattern (or extract a small helper) to any other logger calls (e.g. `logRequestSucceeded`, `logRequestFailed`) that currently interpolate `req.queryParams.query` directly into the description or message so that all of them use the truncated preview instead of the full AQL string.
</issue_to_address>
### Comment 3
<location> `core/database/foxx/tests/proj_router.test.js:10` </location>
<code_context>
+
+const proj_base_url = `${baseUrl}/prj`;
+
+describe("unit_proj_router: test project create endpoint", () => {
+ beforeEach(() => {
+ const collections = [
</code_context>
<issue_to_address>
**suggestion (testing):** Test suite name and structure no longer match the scope (all proj_router endpoints, not just create).
This suite now covers `create`, `update`, `view`, `list`, `search`, `delete`, and `get_role`, so the `describe` name is misleading. Please rename it to reflect all proj_router routes (e.g. `"proj_router routes"`), and consider splitting into separate `describe` blocks per endpoint (`/create`, `/update`, etc.) if the file continues to grow for better test organization.
Suggested implementation:
```javascript
describe("unit_proj_router: proj_router routes", () => {
```
To fully implement your suggestion about structure, you should also:
1. Group tests inside this top-level `describe` into nested `describe` blocks per endpoint, for example:
- `describe("POST /create", () => { ... })`
- `describe("PATCH /update", () => { ... })`
- `describe("GET /view", () => { ... })`
- `describe("GET /list", () => { ... })`
- `describe("GET /search", () => { ... })`
- `describe("DELETE /delete", () => { ... })`
- `describe("GET /get_role", () => { ... })`
2. Move existing tests into the matching nested `describe` based on which endpoint they exercise.
3. If the file continues to grow, consider further nesting by behavior (e.g. success vs error cases) inside each endpoint-specific `describe`.
</issue_to_address>
### Comment 4
<location> `core/database/foxx/tests/proj_router.test.js:370-379` </location>
<code_context>
+ it("should search projects using a provided AQL query", () => {
</code_context>
<issue_to_address>
**suggestion (testing):** Add `/prj/search` tests for invalid AQL or unauthorized clients to exercise error handling and logging.
Because this endpoint executes arbitrary AQL, please also add:
- A test with malformed AQL to verify the route handles errors gracefully (no crashes, expected error response).
- A test with an invalid/unauthorized `client` to confirm access control behavior.
These will exercise the failure path and cover the new `logRequestFailure` behavior for this endpoint.
Suggested implementation:
```javascript
it("should search projects using a provided AQL query", () => {
// ------------------------------------------------------------------
// Arrange
// ------------------------------------------------------------------
db.u.save({ _key: "search_user", is_admin: false });
db.p.save({
_key: "search_proj1",
title: "Alpha Project",
desc: "First searchable project",
ct: 1,
});
db.p.save({
_key: "search_proj2",
title: "Beta Project",
desc: "Second searchable project",
ct: 1,
});
db.p.save({
_key: "other_proj",
title: "Unrelated Project",
desc: "Should not be returned by search",
ct: 1,
});
const validClient = "search_user";
const searchBody = {
client: validClient,
aql: `
FOR p IN p
FILTER LIKE(p.title, @term, true)
SORT p.title ASC
RETURN { id: CONCAT("p/", p._key), title: p.title }
`,
bindVars: {
term: "%Project%",
},
};
// ------------------------------------------------------------------
// Act
// ------------------------------------------------------------------
const response = request.post("/prj/search", searchBody);
// ------------------------------------------------------------------
// Assert
// ------------------------------------------------------------------
expect(response.status).to.equal(200);
const body = JSON.parse(response.body);
const ids = body.map((p) => p.id);
expect(ids).to.include.members(["p/search_proj1", "p/search_proj2"]);
expect(ids).to.not.include("p/other_proj");
});
it("should handle malformed AQL for /prj/search without crashing and return an error response", () => {
// ------------------------------------------------------------------
// Arrange
// ------------------------------------------------------------------
db.u.save({ _key: "search_user_malformed", is_admin: false });
const client = "search_user_malformed";
// Intentionally malformed AQL (missing RETURN and invalid syntax)
const malformedBody = {
client,
aql: "FOR p IN p FILTER p.title == @title INVALID_SYNTAX",
bindVars: {
title: "Alpha Project",
},
};
// ------------------------------------------------------------------
// Act
// ------------------------------------------------------------------
const response = request.post("/prj/search", malformedBody);
// ------------------------------------------------------------------
// Assert
// ------------------------------------------------------------------
// Expect a 400-series error (bad request / invalid query) and a JSON error payload
expect(response.status).to.be.within(400, 499);
const body = JSON.parse(response.body);
expect(body).to.have.property("error", true);
expect(body).to.have.property("code");
expect(body).to.have.property("errorMessage");
});
it("should reject unauthorized clients for /prj/search and log the failure", () => {
// ------------------------------------------------------------------
// Arrange
// ------------------------------------------------------------------
// No user / client record created for this key to simulate unauthorized client
const unauthorizedClient = "nonexistent_client";
const searchBody = {
client: unauthorizedClient,
aql: `
FOR p IN p
RETURN { id: CONCAT("p/", p._key), title: p.title }
`,
bindVars: {},
};
// ------------------------------------------------------------------
// Act
// ------------------------------------------------------------------
const response = request.post("/prj/search", searchBody);
// ------------------------------------------------------------------
// Assert
// ------------------------------------------------------------------
// Expect 401/403 for unauthorized access
expect([401, 403]).to.include(response.status);
const body = JSON.parse(response.body);
expect(body).to.have.property("error", true);
expect(body).to.have.property("code");
expect(body).to.have.property("errorMessage");
// NOTE: We cannot assert directly on logging from here, but hitting this
// failure path will exercise logRequestFailure for this endpoint.
});
```
The edit above assumes:
1. There is a `request` test helper with a `post(path, body)` method returning `{ status, body }`, matching how other tests in this file call the Foxx service. If your helper has a different name or signature (e.g. `foxx.post`, `request("/prj/search", "post", body)`), adjust the calls accordingly.
2. Project collection is named `p` and user collection is `u`, consistent with `db.p.save` and `db.u.save` in the snippet.
3. The error responses for malformed AQL and unauthorized clients follow the same `{ error: true, code, errorMessage }` structure used elsewhere in your API tests. If your error payload shape differs, update the assertions to match your existing conventions.
4. If your router returns a specific status code for invalid AQL (e.g. always `400`) or unauthorized clients (e.g. always `403`), tighten the `expect` assertions from ranges/sets to the exact status codes used by the implementation.
</issue_to_address>
### Comment 5
<location> `core/database/foxx/tests/proj_router.test.js:420-429` </location>
<code_context>
+ it("should enqueue a project delete task when client is authorized", () => {
</code_context>
<issue_to_address>
**suggestion (testing):** Strengthen `/prj/delete` tests with unauthorized and partial-failure scenarios.
The current test only covers the authorized path and asserts a delete task is enqueued while the project still exists (async behavior).
To round out coverage:
- Add a case where the client is not an admin/owner and assert the route returns an error and no delete task is created.
- Optionally, add a case with a mix of valid and invalid project IDs and assert the intended behavior (all-or-nothing vs partial enqueue).
This will more clearly specify the delete semantics and logging in tests.
Suggested implementation:
```javascript
expect(body).to.be.an("array");
expect(body).to.include.members(["p/search_proj1", "p/search_proj2"]);
});
it("should not enqueue a project delete task when client is unauthorized", () => {
// ------------------------------------------------------------------
// Arrange
// ------------------------------------------------------------------
// Non-admin, non-owner user
db.u.save({ _key: "delete_user", is_admin: false });
// Project owned by someone else (or with no explicit owner)
db.p.save({
_key: "delete_proj_unauth",
title: "Do Not Delete",
desc: "Project should not be deleted by unauthorized user",
ct: 1
});
const tasks = db._collection("proj_delete_tasks") || db._create("proj_delete_tasks");
const initialTaskCount = tasks.count();
// ------------------------------------------------------------------
// Act
// ------------------------------------------------------------------
// NOTE: adjust the auth / session setup here to match the rest of the file.
// The important part is that the request is made as `delete_user`.
const response = request.post(`${baseUrl}/prj/delete`, {
body: JSON.stringify({ projectKeys: ["delete_proj_unauth"] }),
headers: {
"content-type": "application/json",
"x-user-key": "delete_user"
}
});
// ------------------------------------------------------------------
// Assert
// ------------------------------------------------------------------
expect(response.statusCode).to.be.oneOf([401, 403]);
// Project must still exist
const project = db.p.document("delete_proj_unauth");
expect(project).to.exist;
expect(project._key).to.equal("delete_proj_unauth");
// No delete task enqueued
expect(tasks.count()).to.equal(initialTaskCount);
// Clean up
tasks.truncate();
db._remove("proj_delete_tasks");
db.p.remove("delete_proj_unauth");
db.u.remove("delete_user");
});
it("should handle a mix of valid and invalid project IDs according to delete semantics", () => {
// ------------------------------------------------------------------
// Arrange
// ------------------------------------------------------------------
db.u.save({ _key: "delete_admin_mixed", is_admin: true });
db.p.save({
_key: "delete_proj_valid_1",
title: "Valid Delete 1",
desc: "First valid project to be deleted",
ct: 1
});
db.p.save({
_key: "delete_proj_valid_2",
title: "Valid Delete 2",
desc: "Second valid project to be deleted",
ct: 1
});
const tasks = db._collection("proj_delete_tasks") || db._create("proj_delete_tasks");
tasks.truncate();
const payload = {
projectKeys: [
"delete_proj_valid_1",
"delete_proj_valid_2",
"non_existent_proj"
]
};
// ------------------------------------------------------------------
// Act
// ------------------------------------------------------------------
const response = request.post(`${baseUrl}/prj/delete`, {
body: JSON.stringify(payload),
headers: {
"content-type": "application/json",
"x-user-key": "delete_admin_mixed"
}
});
// ------------------------------------------------------------------
// Assert
// ------------------------------------------------------------------
// The semantics here may be "all-or-nothing" or "best-effort".
// The assertions below assume "all-or-nothing":
// - the route responds with an error
// - no delete tasks are enqueued
// - all valid projects remain.
//
// If the intended semantics are different, adjust the expectations
// accordingly in this test.
expect(response.statusCode).to.be.within(400, 499);
// No delete tasks enqueued for mixed validity request
expect(tasks.count()).to.equal(0);
// All valid projects must still exist
const valid1 = db.p.document("delete_proj_valid_1");
const valid2 = db.p.document("delete_proj_valid_2");
expect(valid1).to.exist;
expect(valid2).to.exist;
// Clean up
tasks.truncate();
db._remove("proj_delete_tasks");
db.p.remove("delete_proj_valid_1");
db.p.remove("delete_proj_valid_2");
db.u.remove("delete_admin_mixed");
});
it("should enqueue a project delete task when client is authorized", () => {
// ------------------------------------------------------------------
// Arrange
// ------------------------------------------------------------------
db.u.save({ _key: "delete_admin", is_admin: true });
db.p.save({
_key: "delete_proj1",
title: "Delete Me",
desc: "Project to be deleted",
ct: 1,
```
The new tests assume several conventions that must be aligned with the rest of `proj_router.test.js` and the Foxx app:
1. **HTTP client / base URL**
- Replace `request.post` and `baseUrl` with whatever helper the file actually uses to call the Foxx service (e.g. `arango.POST`, `frontend.postJson`, etc.).
- Ensure the `body` / options structure matches the other tests in this file (e.g. `{ json: payload }` vs manual stringify).
2. **Authentication / user selection**
- The tests currently set an `"x-user-key"` header; update this to the actual mechanism used in the test suite to act as a particular user (token header, query param, context helper, etc.).
3. **Task collection name**
- The tests use a collection named `"proj_delete_tasks"` as the queue for delete tasks. Replace this with the real queue collection used by `/prj/delete`, and remove any `db._create` / `db._remove` calls if the collection is managed elsewhere in the test setup/teardown.
4. **Delete semantics**
- The mixed-validity test currently encodes an **all-or-nothing** expectation (error + no tasks + valid projects remain). If the intended semantics are **partial enqueue**, adjust:
- the expected `statusCode`,
- the assertions on `tasks.count()` and content,
- and optionally assert which project keys were enqueued.
</issue_to_address>
### Comment 6
<location> `core/database/foxx/tests/proj_router.test.js:475-484` </location>
<code_context>
+ it("should return the correct project role for client or subject", () => {
</code_context>
<issue_to_address>
**suggestion (testing):** Add `/prj/get_role` tests for non-existent projects and users to validate error handling and logging.
The current test covers roles for owner (3), admin (2), and member (1). To strengthen coverage, please also:
- Add a case where `id` is a non-existent project and assert the route returns the correct error and triggers the failure logging path.
- Add a case where the subject/client has no relationship to the project and assert the role is `0` (or that an error is thrown, depending on the contract).
This will ensure role resolution and error handling are fully exercised.
Suggested implementation:
```javascript
it("should return the correct project role for client or subject", () => {
// ------------------------------------------------------------------
// arrange
// ------------------------------------------------------------------
db.u.save({ _key: "proj_owner", is_admin: false });
db.u.save({ _key: "proj_admin", is_admin: false });
db.u.save({ _key: "proj_member", is_admin: false });
db.u.save({ _key: "proj_stranger", is_admin: false });
db.p.save({
_key: "role_proj",
});
// existing happy-path coverage: owner/admin/member
// owner
let response = request({
method: "POST",
url: baseUrl + "/prj/get_role",
body: {
id: "role_proj",
subject: "proj_owner",
},
json: true,
});
expect(response.statusCode).to.equal(200);
expect(response.json).to.be.an("object");
expect(response.json).to.have.property("role", 3);
// admin
response = request({
method: "POST",
url: baseUrl + "/prj/get_role",
body: {
id: "role_proj",
subject: "proj_admin",
},
json: true,
});
expect(response.statusCode).to.equal(200);
expect(response.json).to.be.an("object");
expect(response.json).to.have.property("role", 2);
// member
response = request({
method: "POST",
url: baseUrl + "/prj/get_role",
body: {
id: "role_proj",
subject: "proj_member",
},
json: true,
});
expect(response.statusCode).to.equal(200);
expect(response.json).to.be.an("object");
expect(response.json).to.have.property("role", 1);
// ------------------------------------------------------------------
// error handling: non‑existent project id
// ------------------------------------------------------------------
const missingProjectResponse = request({
method: "POST",
url: baseUrl + "/prj/get_role",
body: {
id: "does_not_exist",
subject: "proj_owner",
},
json: true,
});
// For a missing project we expect an error response and that the
// failure path (including logging) is triggered.
expect(missingProjectResponse.statusCode).to.be.within(400, 499);
expect(missingProjectResponse.json).to.be.an("object");
expect(missingProjectResponse.json).to.have.property("error", true);
// If the route uses a specific error code/message, assert it here:
// expect(missingProjectResponse.json).to.have.property("code", "project-not-found");
// ------------------------------------------------------------------
// error handling: subject/client has no relationship to project
// ------------------------------------------------------------------
const strangerResponse = request({
method: "POST",
url: baseUrl + "/prj/get_role",
body: {
id: "role_proj",
subject: "proj_stranger",
},
json: true,
});
expect(strangerResponse.statusCode).to.equal(200);
expect(strangerResponse.json).to.be.an("object");
// By contract, a user with no membership should have role 0
// (adjust if the implementation throws instead).
expect(strangerResponse.json).to.have.property("role", 0);
```
The new expectations assume:
- `request` and `baseUrl` are already defined in this test file (as is typical for Foxx route tests).
- `/prj/get_role` returns `{ error: true }` and a 4xx status for missing projects, and `{ role: 0 }` with 200 for users with no relationship to the project.
Please adjust as needed to match the actual contract:
1. If `/prj/get_role` uses different status codes or error payloads, update the corresponding `expect(...)` lines (e.g. asserting a specific `code` or `errorNum`).
2. If the service throws for "no relationship" instead of returning `role: 0`, change the `strangerResponse` assertions to expect the appropriate error status and body shape.
3. If this file uses a different HTTP helper than `request`/`baseUrl`, replace those calls with the local helper while keeping the structure of the three happy-path checks and the two new error-handling checks.
4. Ensure any project–membership edges that the implementation requires are created in the arrange section (if they are not automatically inferred from the user/project documents in your existing tests).
</issue_to_address>
### Comment 7
<location> `core/database/foxx/api/proj_router.js:745` </location>
<code_context>
description: `Delete project(s) and all associated data records and raw data. IDs:${req.body.ids}`,
</code_context>
<issue_to_address>
**security (javascript.express.security.injection.tainted-sql-string):** Detected user input used to manually construct a SQL string. This is usually bad practice because manual construction could accidentally result in a SQL injection. An attacker could use a SQL injection to steal or modify contents of the database. Instead, use a parameterized query which is available by default in most database engines. Alternatively, consider using an object-relational mapper (ORM) such as Sequelize which will protect your queries.
*Source: opengrep*
</issue_to_address>
### Comment 8
<location> `core/database/foxx/api/proj_router.js:768` </location>
<code_context>
description: `Delete project(s) and all associated data records and raw data. IDs:${req.body.ids}`,
</code_context>
<issue_to_address>
**security (javascript.express.security.injection.tainted-sql-string):** Detected user input used to manually construct a SQL string. This is usually bad practice because manual construction could accidentally result in a SQL injection. An attacker could use a SQL injection to steal or modify contents of the database. Instead, use a parameterized query which is available by default in most database engines. Alternatively, consider using an object-relational mapper (ORM) such as Sequelize which will protect your queries.
*Source: opengrep*
</issue_to_address>
### Comment 9
<location> `core/database/foxx/api/proj_router.js:778` </location>
<code_context>
description: `Delete project(s) and all associated data records and raw data. IDs:${req.body.ids}`,
</code_context>
<issue_to_address>
**security (javascript.express.security.injection.tainted-sql-string):** Detected user input used to manually construct a SQL string. This is usually bad practice because manual construction could accidentally result in a SQL injection. An attacker could use a SQL injection to steal or modify contents of the database. Instead, use a parameterized query which is available by default in most database engines. Alternatively, consider using an object-relational mapper (ORM) such as Sequelize which will protect your queries.
*Source: opengrep*
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
JoshuaSBrown
reviewed
Jan 6, 2026
JoshuaSBrown
reviewed
Jan 6, 2026
JoshuaSBrown
reviewed
Jan 6, 2026
JoshuaSBrown
reviewed
Jan 6, 2026
JoshuaSBrown
reviewed
Jan 6, 2026
JoshuaSBrown
reviewed
Jan 6, 2026
JoshuaSBrown
reviewed
Jan 6, 2026
JoshuaSBrown
reviewed
Jan 6, 2026
JoshuaSBrown
reviewed
Jan 6, 2026
JoshuaSBrown
reviewed
Jan 6, 2026
JoshuaSBrown
reviewed
Jan 6, 2026
JoshuaSBrown
reviewed
Jan 6, 2026
JoshuaSBrown
reviewed
Jan 6, 2026
JoshuaSBrown
requested changes
Jan 6, 2026
Collaborator
JoshuaSBrown
left a comment
JoshuaSBrown
left a comment
There was a problem hiding this comment.
Almost there, but still needs a bit more work.
JoshuaSBrown
approved these changes
Jan 12, 2026
megatnt1122
force-pushed
the
refactor-DAPS-1522-Proj-Router-Logging-Improvements
branch
from
589b2af to
8303149
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Ticket
#1522
Description
Logging improvements to proj-router
Tasks
Summary by Sourcery
Improve observability and test coverage for the project router endpoints.
Enhancements:
Build:
Tests: