Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions packages/adapters/src/system/proxy/import/nginx.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,9 @@ function parseUpstreams(config: string): Map<string, string> {
/**
* Turn a raw proxy_pass value into a concrete Openship route target, or reject
* it (so the caller warns and skips) when it can't be resolved to a real
* host:port — an unknown/undeclared upstream, an nginx variable, or a unix
* socket would otherwise produce a vhost that fails `openresty -t`.
* host:port — an unknown/undeclared upstream or an nginx variable would
* otherwise produce a vhost that fails `openresty -t`, and a unix socket one
* that passes it but points at a path the edge can't reach.
*/
function resolveProxyTarget(
proxyPass: string,
Expand All @@ -60,7 +61,13 @@ function resolveProxyTarget(
const scheme = m[1];
const authority = m[2];
const host = authority.replace(/:\d+$/, "");
if (upstreams.has(host)) return { url: `${scheme}${upstreams.get(host)}` };
const upstream = upstreams.get(host);
if (upstream) {
if (/^unix:/i.test(upstream)) {
return { reason: `proxy_pass "${raw}" resolves to upstream "${host}", a unix socket` };
}
return { url: `${scheme}${upstream}` };
}
const isIp = /^\d{1,3}(\.\d{1,3}){3}$/.test(host);
// `localhost` is NOT safe to carry over verbatim: nginx resolves it to ::1
// first, and most app servers bind IPv4 only — so an adopted
Expand Down
14 changes: 14 additions & 0 deletions packages/adapters/src/system/proxy/import/proxy-import.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,20 @@ describe("scanNginx", () => {
expect(res.warnings.some((w) => w.includes("variable"))).toBe(true);
});

test("refuses a unix-socket upstream reached through an `upstream` block", async () => {
const conf = `
upstream app { server unix:/run/app.sock; }
upstream tcp { server 127.0.0.1:9000; }
server { server_name sock.example.com; location / { proxy_pass http://app; } }
server { server_name tcp.example.com; location / { proxy_pass http://tcp; } }
`;
const res = await scanNginx(makeExecutor([["nginx -T", conf]]));
expect(res.sites.some((s) => s.serverNames.includes("sock.example.com"))).toBe(false);
expect(res.warnings.some((w) => w.includes("unix socket"))).toBe(true);
const tcp = res.sites.find((s) => s.serverNames.includes("tcp.example.com"));
expect(tcp?.target).toEqual({ kind: "proxy", url: "http://127.0.0.1:9000" });
});

test("path-routing: keeps EVERY location upstream in `routes`, primary stays `/`", async () => {
// `location /` is declared AFTER `/api` — the primary must still be `/`, and
// the extra upstream is RETAINED (not dropped to a warning) so the edge can
Expand Down