feat(external-upstream): add external upstream service support - #4799
EvanSchleret wants to merge 6 commits into
Conversation
dosubot
Bot
added
size:XXL
bf528c2 to
ecd5937
Compare
|
+1 |
Hey, thanks :) I'm not sure about how PR are selected for implementation. Let's wait and see |
|
@EvanSchleret Can you please review the conflicts? After that I can take a look a the PR |
|
@narcisonunez, yes, I'll do that today. I'll ping you as soon as I've done it. |
|
@narcisonunez I fixed the conflicts. Wish you a happy reviewing haha ! Thank you |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| move: protectedProcedure | ||
| .input(apiMoveExternalUpstream) | ||
| .mutation(async ({ input, ctx }) => { | ||
| await checkServiceAccess(ctx, input.externalUpstreamId, "read"); |
There was a problem hiding this comment.
When a member has read access to an external upstream but lacks service creation permission, move accepts that read permission and directly changes environmentId, allowing the member to relocate the service into another same-organization environment. Every sibling service move endpoint requires service: ["create"] for this state-changing operation. How this was verified: The move path performs only the read check before the database update, while the application and compose move paths require service creation permission.
| try { | ||
| const resolvedAddresses = await lookup(host, { | ||
| all: true, | ||
| verbatim: true, | ||
| }); | ||
|
|
||
| return resolvedAddresses.some((resolved) => | ||
| blockList.check( | ||
| resolved.address, | ||
| resolved.family === 4 ? "ipv4" : "ipv6", | ||
| ), | ||
| ); | ||
| } catch { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
When a permitted user supplies a hostname that initially resolves publicly or fails lookup and later resolves to a blocked address, validation stores the original hostname and Traefik resolves it again when connecting. This allows public routes to reach loopback, link-local, metadata, or another administrator-blocked destination. How this was verified: The validator performs a one-time lookup and treats lookup failure as unblocked, while the stored hostname is emitted unchanged as Traefik's server URL.
Knowledge Base Used: Traefik Networking
| export const DEFAULT_EXTERNAL_UPSTREAM_BLOCKED_CIDRS = [ | ||
| "127.0.0.0/8", | ||
| "169.254.0.0/16", | ||
| "0.0.0.0/8", | ||
| "::1/128", | ||
| "fe80::/10", | ||
| ]; |
There was a problem hiding this comment.
With the default settings, targets in 10.0.0.0/8, 172.16.0.0/12, and 192.168.0.0/16 pass validation and are written directly into Traefik's load-balancer configuration. A permitted user can therefore publish an otherwise internal RFC1918 service through a Dokploy-managed public domain. How this was verified: The code and migration defaults omit every RFC1918 range, and accepted target URLs flow directly into Traefik's upstream server list.
Knowledge Base Used: Global Settings, Docker Registries, and Notifications
| "./db": { | ||
| "import": "./src/db/index.ts", | ||
| "require": "./dist/db/index.cjs.js" | ||
| }, | ||
| "./db/*": { | ||
| "import": "./src/db/*.ts", | ||
| "require": "./dist/db/*.js" | ||
| }, | ||
| "./db/schema": { | ||
| "import": "./src/db/schema/index.ts", | ||
| "require": "./dist/db/schema/index.js" | ||
| }, | ||
| "./db/schema/*": { | ||
| "import": "./src/db/schema/*.ts", | ||
| "require": "./dist/db/schema/*.js" | ||
| }, | ||
| "./db/validations/*": { | ||
| "import": "./src/db/validations/*.ts", | ||
| "require": "./dist/db/validations/*.js" | ||
| }, | ||
| "./emails/*": { | ||
| "import": "./src/emails/*.tsx", | ||
| "require": "./dist/emails/*.js" | ||
| }, | ||
| "./lib/*": { | ||
| "import": "./src/lib/*.ts", | ||
| "require": "./dist/lib/*.js" | ||
| }, | ||
| "./monitoring/*": { | ||
| "import": "./src/monitoring/*.ts", | ||
| "require": "./dist/monitoring/*.js" | ||
| }, | ||
| "./services/*": { | ||
| "import": "./src/services/*.ts", | ||
| "require": "./dist/services/*.js" | ||
| }, | ||
| "./setup/*": { | ||
| "import": "./src/setup/*.ts", | ||
| "require": "./dist/setup/index.cjs.js" | ||
| }, | ||
| "./templates": { | ||
| "import": "./src/templates/index.ts", | ||
| "require": "./dist/templates/index.js" | ||
| }, | ||
| "./templates/*": { | ||
| "import": "./src/templates/*.ts", | ||
| "require": "./dist/templates/*.js" | ||
| }, | ||
| "./types/*": { | ||
| "import": "./src/types/*.ts", | ||
| "require": "./dist/types/*.js" | ||
| }, | ||
| "./constants": { | ||
| "import": "./src/constants/index.ts", | ||
| "require": "./dist/constants.cjs.js" | ||
| }, | ||
| "./utils/*": { | ||
| "import": "./src/utils/*.ts", | ||
| "require": "./dist/utils/*.js" | ||
| }, | ||
| "./utils/ai": { | ||
| "import": "./src/utils/ai/index.ts", | ||
| "require": "./dist/utils/ai/index.js" | ||
| }, | ||
| "./utils/backups": { | ||
| "import": "./src/utils/backups/index.ts", | ||
| "require": "./dist/utils/backups/index.js" | ||
| }, | ||
| "./utils/builders": { | ||
| "import": "./src/utils/builders/index.ts", | ||
| "require": "./dist/utils/builders/index.js" | ||
| }, | ||
| "./utils/restore": { | ||
| "import": "./src/utils/restore/index.ts", | ||
| "require": "./dist/utils/restore/index.js" | ||
| }, | ||
| "./utils/schedules": { | ||
| "import": "./src/utils/schedules/index.ts", | ||
| "require": "./dist/utils/schedules/index.js" | ||
| }, | ||
| "./utils/volume-backups": { | ||
| "import": "./src/utils/volume-backups/index.ts", | ||
| "require": "./dist/utils/volume-backups/index.js" | ||
| }, | ||
| "./verification/*": { | ||
| "import": "./src/verification/*.tsx", | ||
| "require": "./dist/verification/*.js" |
There was a problem hiding this comment.
When server:script or switch:dev runs, switchToSrc.js replaces this expanded export map with the old four-entry map, removing subpaths now used by the application and tests. Subsequent compilation fails to resolve imports such as @dokploy/server/services/permission and @dokploy/server/utils/network/external-upstream; update the export-switching script alongside this map.
Knowledge Base Used: Server Setup and Packaging
|
Hey @narcisonunez, there's already new conflicts. Do you need me to fix them again or is it ok ? Have a good one |
This branch has not been deployed
3 participants
What is this PR about?
This PR adds support for managing external upstream services in Dokploy.
It introduces:
externalUpstreamservice type with schema, service layer, API routes, and project/environment integrationChecklist
Before submitting this PR, please make sure that:
canarybranch.Issues related (if applicable)
N/A
Screenshots (if applicable)
Greptile Summary
The PR introduces external upstream services across persistence, permissions, project integration, domain management, settings, UI, and Traefik configuration.
Confidence Score: 0/5
The PR is not safe to merge until the service-move authorization, outbound target restrictions, DNS validation boundary, and development package exports are corrected.
The changed API permits a read-authorized placement mutation, and the proxy path can expose blocked or private infrastructure because target validation is neither connection-stable nor private-by-default; the development export switch also removes subpaths now required by the application.
Files Needing Attention: apps/dokploy/server/api/routers/external-upstream.ts, packages/server/src/utils/network/external-upstream.ts, packages/server/package.json
Security Review
Three security-boundary issues were identified: read-only users can move upstream services, mutable DNS can bypass target-network validation, and the default policy permits RFC1918 destinations.
Reviews (1): Last reviewed commit: "[autofix.ci] apply automated fixes" | Re-trigger Greptile
Context used (4)