feat: Refactor to Fastify with auto-swagger and MCP support#254
Open
feat: Refactor to Fastify with auto-swagger and MCP support#254
Conversation
- Add split-button dropdown with 'Login as Admin' and 'Login as Standard User' options - Show quick login only when no .env file exists (dev mode) - Auto-create test user and log in with one click - First registered user automatically becomes admin - Show 'No users exist' message on login page when database is empty - Add Playwright tests for quick login and API key creation - Simplify README with developer-focused quick start guide - Update server startup message with clear ready banner
- Replace Express with Fastify 5.x - Add @fastify/swagger for automatic OpenAPI generation from JSON schemas - Add @fastify/swagger-ui at /api/docs - Implement MCP server plugin with container management tools - Port all routes with JSON schema validation: - login/logout, register, reset-password (auth flows) - apikeys, settings (simple CRUD) - users, groups (user management) - sites, nodes, containers (infrastructure) - jobs (async task monitoring with SSE) - external-domains (DNS management) - Create Fastify plugins: - auth.js: requireAuth, requireAdmin, requireLocalhostOrAdmin - flash.js: flash message support for EJS views - load-sites.js: site context for authenticated users - mcp.js: Model Context Protocol server - Preserve dual HTML/JSON content negotiation - Keep existing Sequelize models unchanged - Maintain EJS views with @fastify/view
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
horner
force-pushed
the
feature/fastify-refactor
branch
from
1086bb2 to
2fa96b7
Compare
| }; | ||
| } | ||
|
|
||
| return reply.redirect(redirectUrl); |
Check warning
Code scanning / CodeQL
Server-side URL redirect Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 1 day ago
In general, the fix is to ensure that any redirect URL influenced by user input is strictly validated or normalized before use. The safest approach is to (a) only allow relative paths within the same application, and/or (b) restrict redirects to a known allow-list of paths, so that arbitrary external URLs can never be used.
For this specific code, the best fix with minimal functional change is to enforce that redirectUrl is both a safe relative URL and not an absolute or protocol-relative URL. We can do this where redirectUrl is computed, just before the redirect, by:
- Ensuring the value is a string.
- Rejecting values starting with
http:,https:, or//(case-insensitive). - Continuing to rely on
isSafeRelativeUrlas an additional safeguard.
If any of these checks fail, we fall back to'/'. This keeps the existing behavior (redirect to the requested page when safe, otherwise to/) while making the validation logic explicit in this file.
Concretely:
- In
create-a-container/routes/login.js, replace the block that sets and validatesredirectUrl(lines 283–287) with a more explicit validation that:- Normalizes
request.body.redirectto a string. - Rejects absolute/protocol-relative URLs.
- Uses
isSafeRelativeUrlas a final gate.
No new imports or helper functions are necessary; we only use built-in string methods and the existingisSafeRelativeUrlimport.
- Normalizes
Suggested changeset
1
create-a-container/routes/login.js
| @@ -281,9 +281,25 @@ | ||
| request.session.isAdmin = user.groups?.some(group => group.isAdmin) || false; | ||
|
|
||
| // Return redirect to original page or default to home | ||
| let redirectUrl = request.body.redirect || '/'; | ||
| if (!isSafeRelativeUrl(redirectUrl)) { | ||
| let redirectUrl = request.body.redirect; | ||
|
|
||
| // Ensure redirectUrl is a safe relative URL; otherwise, fall back to home | ||
| if (typeof redirectUrl !== 'string') { | ||
| redirectUrl = '/'; | ||
| } else { | ||
| const trimmed = redirectUrl.trim(); | ||
| const lower = trimmed.toLowerCase(); | ||
| // Disallow absolute/protocol-relative URLs | ||
| if ( | ||
| lower.startsWith('http://') || | ||
| lower.startsWith('https://') || | ||
| lower.startsWith('//') || | ||
| !isSafeRelativeUrl(trimmed) | ||
| ) { | ||
| redirectUrl = '/'; | ||
| } else { | ||
| redirectUrl = trimmed; | ||
| } | ||
| } | ||
|
|
||
| if (request.isApiRequest()) { |
Copilot is powered by AI and may make mistakes. Always verify output.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Refactor create-a-container from Express to Fastify 5.x with auto-swagger and MCP support.
Run: node server-fastify.js