Skip to content

Conversation

@abdotop
Copy link
Member

@abdotop abdotop commented Dec 21, 2025

No description provided.

@abdotop abdotop requested a review from Copilot December 21, 2025 11:59
@abdotop abdotop self-assigned this Dec 21, 2025
@abdotop abdotop linked an issue Dec 21, 2025 that may be closed by this pull request
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This pull request adds a developer-only SQL execution endpoint with token-based authorization for debugging and development purposes. The changes introduce a new internal authentication mechanism and refactor the router to support automatic registration of reserved system routes.

  • Adds DEV_INTERNAL_TOKEN environment variable for securing developer access
  • Implements /api/execute-sql endpoint for executing arbitrary SQL queries with authorization
  • Refactors makeRouter to accept an options object and support optional SQL injection

Reviewed changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
types/router.d.ts Adds ReservedRoutes type to document system routes that can be overridden (documentation, health check, SQL execution)
db/mod.ts Exports Sql type for use in router configuration
api/env.ts Adds DEV_INTERNAL_TOKEN environment variable for developer authentication
api/dev.ts Implements developer authorization function and SQL execution route handler
api/router.ts Refactors makeRouter to accept options object and automatically registers SQL dev route if not overridden
.github/workflows/ga-npm-publish.yml Updates quote style from double to single quotes for consistency

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@abdotop abdotop force-pushed the 5-add-documentation-support-and-sql-execution branch from 2b26d2e to f32aee8 Compare December 21, 2025 12:14
@abdotop abdotop requested a review from Copilot December 21, 2025 13:04
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 7 changed files in this pull request and generated 6 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

api/dev.ts Outdated
if (APP_ENV === 'prod') {
throw new respond.UnauthorizedError({ message: 'Unauthorized access' })
}
// In dev/test with no configured token, allow free access.
Copy link

Copilot AI Dec 21, 2025

Choose a reason for hiding this comment

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

The authorization logic has a potential security issue. When DEV_INTERNAL_TOKEN is not set and APP_ENV is not 'prod', the function allows unrestricted access without any authentication. This means in development and test environments, anyone can execute arbitrary SQL queries without authentication.

While this may be intentional for development convenience, it poses a risk if:

  1. Development or test environments are accessible from untrusted networks
  2. The APP_ENV configuration is accidentally misconfigured
  3. Staging environments are treated as non-production but contain sensitive data

Consider adding an explicit opt-in flag for unauthenticated access or at least logging a warning when this code path is taken.

Suggested change
// In dev/test with no configured token, allow free access.
// In dev/test with no configured token, allow free access, but log a warning.
console.warn(
'[authorizeDevAccess] Unauthenticated developer access allowed because DEV_INTERNAL_TOKEN is not set and APP_ENV is "%s". This should only be enabled in isolated development environments. Request URL: %s',
APP_ENV,
req.url,
)

Copilot uses AI. Check for mistakes.
@abdotop abdotop changed the title Add developer access authorization and SQL execution route Add documentation support and SQL execution Dec 22, 2025
*
* const result = sql`SELECT * FROM users WHERE id = ${1}`.get();
* ```
*/
Copy link
Member

Choose a reason for hiding this comment

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

export type Sql = <
  T extends { [k in string]: unknown } | undefined,
  P extends BindValue | BindParameters | undefined,
>(sqlArr: TemplateStringsArray, ...vars: unknown[]) => {
  get: (params?: P) => T | undefined
  all: (params?: P) => T[]
  run: (params?: P) => void
  value: (params?: P) => T[keyof T][] | undefined
}

export const sql: Sql = (sqlArr, ...vars) => { /*...*/ }

api/env.ts Outdated
* }
* ```
*/
export const DEV_INTERNAL_TOKEN: string = ENV('DEV_INTERNAL_TOKEN', '')
Copy link
Member

Choose a reason for hiding this comment

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

DEVTOOL_ACCESS_TOKEN and rename DEVTOOL_TOKEN to DEVTOOL_REPORT_TOKEN

api/doc.ts Outdated
// of the documentation objects is dynamic and may vary.
// This allows for flexible object structures within the array
// while maintaining type safety at the array level.
output: ARR(OBJ({}), 'API documentation object structure'),
Copy link
Member

Choose a reason for hiding this comment

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

could be:

ARR(OBJ({
        method: ENUM('GET', 'POST', ...),
        path: STR(''),
        requiresAuth: BOOL(''),
        authFunction: STR('name of the auth function'),
        description: STR(''),
        input: OPTIONAL(OBJ({})),
        output: OPTIONAL(OBJ({})),
})

api/dev.ts Outdated
const bearer = auth.toLowerCase().startsWith('bearer ')
? auth.slice(7).trim()
: ''

Copy link
Member

@kigiri kigiri Dec 22, 2025

Choose a reason for hiding this comment

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

if (APP_ENV !== 'prod') return // always open for dev env
 const bearer = auth.toLowerCase().startsWith('bearer ')
    ? auth.slice(7).trim() : ''
if (bearer && bearer === DEV_INTERNAL_TOKEN) return
throw new respond.UnauthorizedError({ message: 'Unauthorized access' })

@abdotop abdotop force-pushed the 5-add-documentation-support-and-sql-execution branch from ddceac8 to 7dcc00d Compare December 24, 2025 09:10
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.

Add documentation support and SQL execution

3 participants