Skip to content

Fix logic gaps: auth refresh tokens, provider routes, and Dockerfile#1

Open
kai-lucky72 wants to merge 1 commit intorootfrom
beta-version-finalism
Open

Fix logic gaps: auth refresh tokens, provider routes, and Dockerfile#1
kai-lucky72 wants to merge 1 commit intorootfrom
beta-version-finalism

Conversation

@kai-lucky72
Copy link
Copy Markdown

i did some finalising about the backend logic and also completed some workflows for users

@qodo-code-review
Copy link
Copy Markdown

Review Summary by Qodo

Fix database creation, route ordering, and add refresh tokens

🐞 Bug fix ✨ Enhancement

Grey Divider

Walkthroughs

Description
• Add automatic database creation in migration script
  - Checks if database exists before running schema
  - Creates database if missing, handles connection errors gracefully
• Reorder provider routes for correct Express routing precedence
  - Move /me/profile and /me/stats routes before /{id} route
  - Prevents parameterized routes from intercepting user-specific endpoints
• Add refresh tokens table to database schema
  - Stores hashed tokens with expiration and revocation status
  - Includes indexes for efficient token lookups
• Fix Dockerfile entry point from server.js to index.js
• Add TypeScript migration script to package.json
Diagram
flowchart LR
  A["Migration Script"] -->|"Check/Create DB"| B["Database"]
  C["Provider Routes"] -->|"Reorder /me/* before /{id}"| D["Correct Routing"]
  E["Schema"] -->|"Add refresh_tokens table"| B
  F["Dockerfile"] -->|"Fix entry point"| G["index.js"]
  H["package.json"] -->|"Add db:migrate:ts"| I["TypeScript Support"]
Loading

Grey Divider

File Changes

1. src/database/migrate.ts ✨ Enhancement +34/-31

Add automatic database creation logic

• Add database existence check and automatic creation before schema migration
• Connect to default postgres database to check if target database exists
• Create database if missing with proper error handling and logging
• Remove commented-out migration tracking code

src/database/migrate.ts


2. src/routes/provider.routes.ts 🐞 Bug fix +73/-72

Reorder routes for correct precedence

• Move /me/profile and /me/stats routes before /{id} route
• Ensures user-specific endpoints are matched before parameterized ID routes
• Maintains all Swagger documentation for both endpoints

src/routes/provider.routes.ts


3. Dockerfile 🐞 Bug fix +1/-1

Fix Docker entry point

• Change CMD entry point from dist/server.js to dist/index.js
• Aligns with actual application entry point

Dockerfile


View more (2)
4. package.json ✨ Enhancement +2/-1

Add TypeScript migration script

• Add db:migrate:ts script for running migrations with TypeScript directly
• Allows development-time migrations without compilation step
• Fix file ending to newline

package.json


5. src/database/schema.sql ✨ Enhancement +13/-0

Add refresh tokens table schema

• Add refresh_tokens table for storing JWT refresh tokens
• Include token hash, user reference, expiration, and revocation status
• Create indexes on token hash and user ID for query performance

src/database/schema.sql


Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Copy Markdown

qodo-code-review Bot commented Mar 22, 2026

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📎 Requirement gaps (0) 📐 Spec deviations (0)

Grey Divider


Action required

1. Unsanitized CREATE DATABASE name 🐞 Bug ⛨ Security
Description
src/database/migrate.ts interpolates config.database.name (DB_NAME) directly into a CREATE DATABASE
statement without identifier escaping/validation, so a name containing a double-quote can break the
SQL and potentially append unintended DDL. This can cause migrations to fail or execute unexpected
statements on the admin connection used for DB creation.
Code

src/database/migrate.ts[R18-37]

+    const dbName = config.database.name || 'hano_db';
+    console.log(`Checking if database "${dbName}" exists...`);
+
+    const client = new Client({
+      host: config.database.host,
+      port: config.database.port,
+      user: config.database.user,
+      password: config.database.password,
+      database: 'postgres', // Connect to default postgres DB first
+    });
+
+    try {
+      await client.connect();
+      const checkDb = await client.query(`SELECT 1 FROM pg_database WHERE datname = $1`, [dbName]);
+
+      if (checkDb.rows.length === 0) {
+        console.log(`Database "${dbName}" not found. Creating it...`);
+        // Cannot use parameterized query for CREATE DATABASE
+        await client.query(`CREATE DATABASE "${dbName}"`);
+        console.log(`✓ Database "${dbName}" created successfully.`);
Evidence
DB_NAME is sourced from the environment with no validation, and is then embedded into CREATE
DATABASE via string interpolation rather than safe identifier-escaping.

src/config/config.ts[78-85]
src/database/migrate.ts[17-37]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`src/database/migrate.ts` constructs `CREATE DATABASE "${dbName}"` using `dbName` from `process.env.DB_NAME` (via `config.database.name`) without validating it as a safe PostgreSQL identifier or escaping embedded quotes. This can break the statement and can enable DDL injection in the migration step.

### Issue Context
PostgreSQL DDL like `CREATE DATABASE` can’t be parameterized like normal value parameters, so the fix should be identifier validation/escaping (not `$1` parameters).

### Fix Focus Areas
- src/database/migrate.ts[17-37]
- src/config/config.ts[78-85]

### What to change
- Add strict validation for `dbName` (e.g., allow only `[A-Za-z_][A-Za-z0-9_]*`), and fail fast with a clear error if invalid.
- Alternatively (or additionally), implement proper identifier escaping (double any embedded `"` to `""`) and still validate length/charset.
- Keep the `SELECT 1 FROM pg_database WHERE datname = $1` check as-is (it’s already parameterized).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

2. Redundant token_hash index 🐞 Bug ➹ Performance
Description
src/database/schema.sql defines refresh_tokens.token_hash as UNIQUE and also creates
idx_refresh_tokens_hash on the same column, resulting in duplicate indexing and unnecessary
write/storage overhead. This adds avoidable cost on refresh token inserts/rotations.
Code

src/database/schema.sql[R249-259]

+CREATE TABLE IF NOT EXISTS refresh_tokens (
+    id UUID PRIMARY KEY DEFAULT uuid_generate_v4(),
+    user_id UUID NOT NULL REFERENCES users(id) ON DELETE CASCADE,
+    token_hash VARCHAR(255) UNIQUE NOT NULL,
+    expires_at TIMESTAMP NOT NULL,
+    revoked BOOLEAN DEFAULT FALSE,
+    created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP
+);
+
+CREATE INDEX IF NOT EXISTS idx_refresh_tokens_hash ON refresh_tokens(token_hash);
+CREATE INDEX IF NOT EXISTS idx_refresh_tokens_user ON refresh_tokens(user_id);
Evidence
The schema simultaneously declares a UNIQUE constraint on token_hash and then creates a separate
index on token_hash, which is redundant for enforcing uniqueness and lookups on that same column.

src/database/schema.sql[249-259]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`refresh_tokens.token_hash` is declared `UNIQUE`, and the schema also creates `idx_refresh_tokens_hash` on the same column. This duplicates indexing work and increases write overhead.

### Issue Context
A `UNIQUE` constraint already provides an index suitable for lookups by `token_hash`.

### Fix Focus Areas
- src/database/schema.sql[249-259]

### What to change
- Remove `CREATE INDEX IF NOT EXISTS idx_refresh_tokens_hash ON refresh_tokens(token_hash);`
- Keep the UNIQUE constraint on `token_hash` (since the model assumes token hashes are unique).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

Comment thread src/database/migrate.ts
Comment on lines +18 to +37
const dbName = config.database.name || 'hano_db';
console.log(`Checking if database "${dbName}" exists...`);

const client = new Client({
host: config.database.host,
port: config.database.port,
user: config.database.user,
password: config.database.password,
database: 'postgres', // Connect to default postgres DB first
});

try {
await client.connect();
const checkDb = await client.query(`SELECT 1 FROM pg_database WHERE datname = $1`, [dbName]);

if (checkDb.rows.length === 0) {
console.log(`Database "${dbName}" not found. Creating it...`);
// Cannot use parameterized query for CREATE DATABASE
await client.query(`CREATE DATABASE "${dbName}"`);
console.log(`✓ Database "${dbName}" created successfully.`);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

1. Unsanitized create database name 🐞 Bug ⛨ Security

src/database/migrate.ts interpolates config.database.name (DB_NAME) directly into a CREATE DATABASE
statement without identifier escaping/validation, so a name containing a double-quote can break the
SQL and potentially append unintended DDL. This can cause migrations to fail or execute unexpected
statements on the admin connection used for DB creation.
Agent Prompt
### Issue description
`src/database/migrate.ts` constructs `CREATE DATABASE "${dbName}"` using `dbName` from `process.env.DB_NAME` (via `config.database.name`) without validating it as a safe PostgreSQL identifier or escaping embedded quotes. This can break the statement and can enable DDL injection in the migration step.

### Issue Context
PostgreSQL DDL like `CREATE DATABASE` can’t be parameterized like normal value parameters, so the fix should be identifier validation/escaping (not `$1` parameters).

### Fix Focus Areas
- src/database/migrate.ts[17-37]
- src/config/config.ts[78-85]

### What to change
- Add strict validation for `dbName` (e.g., allow only `[A-Za-z_][A-Za-z0-9_]*`), and fail fast with a clear error if invalid.
- Alternatively (or additionally), implement proper identifier escaping (double any embedded `"` to `""`) and still validate length/charset.
- Keep the `SELECT 1 FROM pg_database WHERE datname = $1` check as-is (it’s already parameterized).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant