fix(adapters): refuse a unix-socket upstream in the nginx importer - #412
Open
shuvamk wants to merge 1 commit into
Open
fix(adapters): refuse a unix-socket upstream in the nginx importer#412shuvamk wants to merge 1 commit into
shuvamk wants to merge 1 commit into
Conversation
`resolveProxyTarget` refuses `proxy_pass http://unix:/run/app.sock:/` outright,
but the same socket reached through an `upstream` block was migrated. The
upstream branch substituted the block's `server` value into the scheme without
looking at it, so `unix:/run/app.sock` became the route target
`http://unix:/run/app.sock`.
Both spellings are the same config to nginx. Measured on `nginx:1.27-alpine`,
`nginx -t` is `Syntax OK` for the upstream form the operator wrote, and a
request through either `proxy_pass http://unix:/run/app.sock` or
`proxy_pass http://unix:/run/app.sock:/` logs the identical
`upstream: "http://unix:/run/app.sock:/foo"`. Only which one the operator typed
decided whether the site migrated.
upstream app { server unix:/run/app.sock; }
server { server_name sock.example.com; location / { proxy_pass http://app; } }
before: sites [{ serverNames: ["sock.example.com"],
target: { kind: "proxy", url: "http://unix:/run/app.sock" } }]
warnings []
after: sites [], warning
`nginx: sock.example.com / — proxy_pass "http://app" resolves to upstream "app", a unix socket (skipped)`
Nothing downstream catches it. Run end to end through `scanNginx` ->
`registerImportedSites` with a real `NginxProvider` over a fake executor, `main`
returns `["sock.example.com"]`, collects no warning, writes a vhost containing
`proxy_pass http://unix:/run/app.sock;`, and logs
Migrated sock.example.com → http://unix:/run/app.sock
`assertValidUpstream` accepts that URL as an http URL, and `openresty -t` on the
edge's own base image (`openresty/openresty:1.27.1.1-alpine`) accepts the
directive. Of the three shapes this function rejects, the unix socket is the
only one that passes `nginx -t` at all - `http://ghost` is `host not found in
upstream` and `http://$backend` is `unknown "backend" variable` - so the
function's doc comment is corrected alongside the fix.
The socket is not reachable from the edge that now serves it:
EDGE_CONTAINER_MOUNTS mounts nothing under /run, so the host answers 502. This
is the takeover path, where the operator's nginx is already stopped, and the
domain is reported as successfully migrated.
Warn and skip is what the direct form already does, and what the caddy importer
does for both of its unix-socket shapes. The upstream branch now applies the
same test to the value it resolved.
A 21-config corpus through `scanNginx` differs on exactly the two
unix-socket-via-upstream cases. The other 19 - including the direct unix form,
which `main` already refuses - are byte-identical.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
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.
Summary
The nginx importer refuses
proxy_pass http://unix:/run/app.sock:/outright, but migrates the same socket when it is reached through anupstreamblock — as the route targethttp://unix:/run/app.sock.Motivation
resolveProxyTargetsubstitutes the block'sservervalue into the scheme without looking at it. To nginx the two spellings are one config — measured onnginx:1.27-alpine,proxy_pass http://unix:/run/app.sockand…:/log the identicalupstream: "http://unix:/run/app.sock:/foo"— so only which one the operator typed decided whether the site migrated.Nothing downstream catches it:
assertValidUpstreamacceptshttp://unix:/…as an http URL, andopenresty -ton the edge's own base image acceptsproxy_pass http://unix:/run/app.sock;. But that socket is unreachable from the edge now serving it —EDGE_CONTAINER_MOUNTSmounts nothing under/run— so the host answers 502. On the takeover path the operator's nginx is already stopped, and the domain is reported as migrated.Warn and skip is what the direct form already does, and what the caddy importer does for both of its unix-socket shapes.
Related issue
None.
Changes
packages/adapters— the upstream branch ofresolveProxyTargetapplies the existing unix-socket test to the value it resolved.proxy-import.test.ts.Verification
A 21-config corpus through
scanNginxdiffers on exactly the two unix-socket-via-upstream cases.Happy to migrate these instead of skipping them, if you'd rather — that needs the edge to see the socket path.
Checklist
bun run testandbun run --cwd packages/adapters lintpass; both files carry pre-existing prettier drift onmain, so I hand-formatted my own lines instead of running--writeover them