Skip to content

Commit 4309c16

Browse files
authored
Fail fast when HTTPS is requested but certificates are missing (#1640)
- Add ServerConfigError and throw when cert files are missing - Catch ServerConfigError in node-server.ts with a clean fatal log - Fix docker-entrypoint.sh: add set -e, fix bracket syntax, fix duplicate check - Fix csr.conf trailing space so sed substitution matches - Add docker-entrypoint.sh integration tests - Add config-pipeline integration tests (shell → dotenv → Zod → server config) - Add ServerConfigError message content tests - Add grep-safe .env output tests for process-environment.sh - Add test for openssl failure propagation in setup-ssl - Read real cert/csr config files in setup-ssl tests Closes #1634
1 parent f72b9b1 commit 4309c16

File tree

11 files changed

+503
-67
lines changed

11 files changed

+503
-67
lines changed

.gitignore

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,3 +8,6 @@
88

99
# TypeScript build cache manifests
1010
*.tsbuildinfo
11+
12+
# Local planning docs
13+
plans/

docker-entrypoint.sh

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,18 @@
11
#!/bin/sh
2+
set -e
23

34
./process-environment.sh
45

56
CONFIGURATION_FOLDER_PATH=${CONFIGURATION_FOLDER_PATH:-"./packages/graph-explorer/"}
6-
PROXY_SERVER_HTTPS_CONNECTION_VALUE=$(grep -e 'PROXY_SERVER_HTTPS_CONNECTION' $CONFIGURATION_FOLDER_PATH/.env | cut -d "=" -f 2)
77

8-
if [ -n "$PROXY_SERVER_HTTPS_CONNECTION_VALUE" ] && [ "$PROXY_SERVER_HTTPS_CONNECTION_VALUE" == "true" ]; then
8+
if [ ! -f "$CONFIGURATION_FOLDER_PATH/.env" ]; then
9+
echo "Expected .env file not found at $CONFIGURATION_FOLDER_PATH/.env" >&2
10+
exit 1
11+
fi
12+
13+
PROXY_SERVER_HTTPS_CONNECTION_VALUE=$(grep -e '^PROXY_SERVER_HTTPS_CONNECTION=' "$CONFIGURATION_FOLDER_PATH/.env" | cut -d "=" -f 2 || true)
14+
15+
if [ -n "$PROXY_SERVER_HTTPS_CONNECTION_VALUE" ] && [ "$PROXY_SERVER_HTTPS_CONNECTION_VALUE" = "true" ]; then
916
CERT_DIR=/graph-explorer/packages/graph-explorer-proxy-server/cert-info \
1017
HOST="$HOST" \
1118
./setup-ssl.sh
@@ -14,4 +21,5 @@ else
1421
fi
1522

1623
echo "Starting graph explorer..."
24+
# Stubbed in tests — update docker-entrypoint.test.ts if changing
1725
cd /graph-explorer/packages/graph-explorer-proxy-server && NODE_ENV=production node dist/node-server.js

packages/graph-explorer-proxy-server/cert-info/csr.conf

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,4 +17,4 @@ subjectAltName = @alt_names
1717

1818
[ alt_names ]
1919
# Filled in by Dockerfile
20-
DNS.1 =
20+
DNS.1 =
Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
import { execFileSync } from "child_process";
2+
import dotenv from "dotenv";
3+
import fs from "fs";
4+
import os from "os";
5+
import path from "path";
6+
7+
import { parseEnvironmentValues } from "./env.js";
8+
import { proxyServerRoot } from "./paths.js";
9+
import { resolveServerConfig, ServerConfigError } from "./server-config.js";
10+
11+
const expectedKeyPath = path.join(proxyServerRoot, "cert-info/server.key");
12+
const expectedCertPath = path.join(proxyServerRoot, "cert-info/server.crt");
13+
14+
const processEnvScriptPath = path.resolve(
15+
import.meta.dirname,
16+
"../../../process-environment.sh",
17+
);
18+
19+
function runPipeline(
20+
workDir: string,
21+
env: Record<string, string> = {},
22+
presetEnvVars: Record<string, string> = {},
23+
) {
24+
const configFolder =
25+
env.CONFIGURATION_FOLDER_PATH ?? "./packages/graph-explorer/";
26+
const resolvedConfigFolder = path.resolve(workDir, configFolder);
27+
fs.mkdirSync(resolvedConfigFolder, { recursive: true });
28+
29+
// Pre-seed .env with vars the shell script doesn't write
30+
if (Object.keys(presetEnvVars).length > 0) {
31+
const lines = Object.entries(presetEnvVars)
32+
.map(([k, v]) => `${k}=${v}`)
33+
.join("\n");
34+
fs.writeFileSync(path.join(resolvedConfigFolder, ".env"), lines + "\n");
35+
}
36+
37+
execFileSync("sh", [processEnvScriptPath], {
38+
cwd: workDir,
39+
env: { ...env, PATH: process.env.PATH },
40+
});
41+
42+
const envFilePath = path.join(resolvedConfigFolder, ".env");
43+
const envFileContent = fs.readFileSync(envFilePath, "utf-8");
44+
const parsed = dotenv.parse(envFileContent);
45+
return parseEnvironmentValues(parsed);
46+
}
47+
48+
describe("config pipeline: shell → dotenv → Zod → server config", () => {
49+
let workDir: string;
50+
51+
beforeEach(() => {
52+
workDir = fs.mkdtempSync(path.join(os.tmpdir(), "ge-pipeline-test-"));
53+
});
54+
55+
afterEach(() => {
56+
fs.rmSync(workDir, { recursive: true, force: true });
57+
vi.restoreAllMocks();
58+
});
59+
60+
it("HTTPS enabled by default flows through to server config", () => {
61+
const env = runPipeline(workDir);
62+
expect(env.PROXY_SERVER_HTTPS_CONNECTION).toBe(true);
63+
64+
vi.spyOn(fs, "existsSync").mockImplementation(
65+
p => p === expectedKeyPath || p === expectedCertPath,
66+
);
67+
const config = resolveServerConfig(env);
68+
expect(config.useHttps).toBe(true);
69+
expect(config.port).toBe(443);
70+
});
71+
72+
it("HTTPS disabled flows through to HTTP config", () => {
73+
const env = runPipeline(workDir, {
74+
PROXY_SERVER_HTTPS_CONNECTION: "false",
75+
});
76+
77+
const config = resolveServerConfig(env);
78+
expect(config.useHttps).toBe(false);
79+
expect(config.port).toBe(80);
80+
});
81+
82+
it("Neptune Notebook forces HTTPS off", () => {
83+
const env = runPipeline(workDir, { NEPTUNE_NOTEBOOK: "true" });
84+
85+
const config = resolveServerConfig(env);
86+
expect(config.useHttps).toBe(false);
87+
});
88+
89+
it("HTTPS enabled but certs missing throws ServerConfigError", () => {
90+
const env = runPipeline(workDir);
91+
expect(env.PROXY_SERVER_HTTPS_CONNECTION).toBe(true);
92+
93+
expect(() => resolveServerConfig(env)).toThrow(ServerConfigError);
94+
});
95+
96+
it("custom HTTP port flows through", () => {
97+
const env = runPipeline(
98+
workDir,
99+
{ PROXY_SERVER_HTTPS_CONNECTION: "false" },
100+
{ PROXY_SERVER_HTTP_PORT: "8080" },
101+
);
102+
103+
const config = resolveServerConfig(env);
104+
expect(config.port).toBe(8080);
105+
});
106+
});
Lines changed: 194 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,194 @@
1+
import { execFileSync } from "child_process";
2+
import fs from "fs";
3+
import os from "os";
4+
import path from "path";
5+
6+
const originalScriptPath = path.resolve(
7+
import.meta.dirname,
8+
"../../../docker-entrypoint.sh",
9+
);
10+
11+
function setupWorkDir() {
12+
const workDir = fs.mkdtempSync(path.join(os.tmpdir(), "ge-entrypoint-test-"));
13+
const configDir = path.join(workDir, "packages", "graph-explorer");
14+
fs.mkdirSync(configDir, { recursive: true });
15+
16+
// Create modified entrypoint with stubbed last line
17+
const script = fs.readFileSync(originalScriptPath, "utf-8");
18+
const serverStartLine =
19+
"cd /graph-explorer/packages/graph-explorer-proxy-server && NODE_ENV=production node dist/node-server.js";
20+
if (!script.includes(serverStartLine)) {
21+
throw new Error(
22+
"docker-entrypoint.sh no longer contains the expected server start line. Update the test stub.",
23+
);
24+
}
25+
const modifiedScript = script.replace(
26+
serverStartLine,
27+
'echo "SERVER_STARTED"',
28+
);
29+
const scriptPath = path.join(workDir, "docker-entrypoint.sh");
30+
fs.writeFileSync(scriptPath, modifiedScript, { mode: 0o755 });
31+
32+
// Default stubs
33+
fs.writeFileSync(
34+
path.join(workDir, "process-environment.sh"),
35+
"#!/bin/sh\nexit 0\n",
36+
{ mode: 0o755 },
37+
);
38+
fs.writeFileSync(
39+
path.join(workDir, "setup-ssl.sh"),
40+
'#!/bin/sh\ntouch ./ssl-called\necho "$HOST" > ./host-value\n',
41+
{ mode: 0o755 },
42+
);
43+
44+
return { workDir, configDir, scriptPath };
45+
}
46+
47+
function writeEnv(configDir: string, content: string) {
48+
fs.writeFileSync(path.join(configDir, ".env"), content);
49+
}
50+
51+
function runScript(
52+
workDir: string,
53+
scriptPath: string,
54+
env: Record<string, string> = {},
55+
): { exitCode: number; stdout: string; stderr: string } {
56+
try {
57+
const stdout = execFileSync("sh", [scriptPath], {
58+
cwd: workDir,
59+
env: { PATH: process.env.PATH, ...env },
60+
encoding: "utf-8",
61+
stdio: ["pipe", "pipe", "pipe"],
62+
});
63+
return { exitCode: 0, stdout, stderr: "" };
64+
} catch (error: unknown) {
65+
const e = error as { status: number; stdout: string; stderr: string };
66+
return {
67+
exitCode: e.status,
68+
stdout: e.stdout ?? "",
69+
stderr: e.stderr ?? "",
70+
};
71+
}
72+
}
73+
74+
describe("docker-entrypoint.sh", () => {
75+
let workDir: string;
76+
let configDir: string;
77+
let scriptPath: string;
78+
79+
beforeEach(() => {
80+
({ workDir, configDir, scriptPath } = setupWorkDir());
81+
});
82+
83+
afterEach(() => {
84+
fs.rmSync(workDir, { recursive: true, force: true });
85+
});
86+
87+
it("fails when .env file is missing", () => {
88+
// Don't write any .env file
89+
90+
const { exitCode, stderr } = runScript(workDir, scriptPath);
91+
expect(exitCode).not.toBe(0);
92+
expect(stderr).toContain(".env");
93+
});
94+
95+
it("set -e propagates process-environment.sh failure", () => {
96+
fs.writeFileSync(
97+
path.join(workDir, "process-environment.sh"),
98+
"#!/bin/sh\nexit 1\n",
99+
{ mode: 0o755 },
100+
);
101+
102+
const { exitCode } = runScript(workDir, scriptPath);
103+
expect(exitCode).not.toBe(0);
104+
});
105+
106+
it("set -e propagates setup-ssl.sh failure", () => {
107+
writeEnv(configDir, "PROXY_SERVER_HTTPS_CONNECTION=true\n");
108+
fs.writeFileSync(
109+
path.join(workDir, "setup-ssl.sh"),
110+
"#!/bin/sh\nexit 1\n",
111+
{ mode: 0o755 },
112+
);
113+
114+
const { exitCode, stdout } = runScript(workDir, scriptPath);
115+
expect(exitCode).not.toBe(0);
116+
expect(stdout).not.toContain("Starting graph explorer");
117+
});
118+
119+
it("calls setup-ssl.sh when HTTPS is true", () => {
120+
writeEnv(configDir, "PROXY_SERVER_HTTPS_CONNECTION=true\n");
121+
122+
const { exitCode } = runScript(workDir, scriptPath, {
123+
HOST: "localhost",
124+
});
125+
126+
expect(exitCode).toBe(0);
127+
expect(fs.existsSync(path.join(workDir, "ssl-called"))).toBe(true);
128+
});
129+
130+
it("skips setup-ssl.sh when HTTPS is false", () => {
131+
writeEnv(configDir, "PROXY_SERVER_HTTPS_CONNECTION=false\n");
132+
133+
const { exitCode, stdout } = runScript(workDir, scriptPath);
134+
135+
expect(exitCode).toBe(0);
136+
expect(fs.existsSync(path.join(workDir, "ssl-called"))).toBe(false);
137+
expect(stdout).toContain("SSL disabled");
138+
});
139+
140+
it("skips setup-ssl.sh when PROXY_SERVER_HTTPS_CONNECTION is absent", () => {
141+
writeEnv(configDir, "LOG_LEVEL=info\n");
142+
143+
const { exitCode, stdout } = runScript(workDir, scriptPath);
144+
145+
expect(exitCode).toBe(0);
146+
expect(fs.existsSync(path.join(workDir, "ssl-called"))).toBe(false);
147+
expect(stdout).toContain("SSL disabled");
148+
});
149+
150+
it("grep ignores commented-out lines", () => {
151+
writeEnv(configDir, "# PROXY_SERVER_HTTPS_CONNECTION=true\n");
152+
153+
const { exitCode } = runScript(workDir, scriptPath);
154+
155+
expect(exitCode).toBe(0);
156+
expect(fs.existsSync(path.join(workDir, "ssl-called"))).toBe(false);
157+
});
158+
159+
it("grep ignores similarly-named variables", () => {
160+
writeEnv(configDir, "GRAPH_EXP_PROXY_SERVER_HTTPS_CONNECTION=true\n");
161+
162+
const { exitCode } = runScript(workDir, scriptPath);
163+
164+
expect(exitCode).toBe(0);
165+
expect(fs.existsSync(path.join(workDir, "ssl-called"))).toBe(false);
166+
});
167+
168+
it("passes HOST to setup-ssl.sh", () => {
169+
writeEnv(configDir, "PROXY_SERVER_HTTPS_CONNECTION=true\n");
170+
171+
runScript(workDir, scriptPath, { HOST: "my-test-host" });
172+
173+
const hostValue = fs
174+
.readFileSync(path.join(workDir, "host-value"), "utf-8")
175+
.trim();
176+
expect(hostValue).toBe("my-test-host");
177+
});
178+
179+
it("uses custom CONFIGURATION_FOLDER_PATH", () => {
180+
const customDir = path.join(workDir, "custom-config");
181+
fs.mkdirSync(customDir, { recursive: true });
182+
fs.writeFileSync(
183+
path.join(customDir, ".env"),
184+
"PROXY_SERVER_HTTPS_CONNECTION=false\n",
185+
);
186+
187+
const { exitCode, stdout } = runScript(workDir, scriptPath, {
188+
CONFIGURATION_FOLDER_PATH: customDir,
189+
});
190+
191+
expect(exitCode).toBe(0);
192+
expect(stdout).toContain("SSL disabled");
193+
});
194+
});

packages/graph-explorer-proxy-server/src/node-server.ts

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { parseEnvironmentValues } from "./env.js";
66
import { handleError } from "./error-handler.js";
77
import { createLogger } from "./logging.js";
88
import { clientRoot, isDirectory } from "./paths.js";
9-
import { resolveServerConfig } from "./server-config.js";
9+
import { resolveServerConfig, ServerConfigError } from "./server-config.js";
1010
import { createServer } from "./server.js";
1111

1212
// Load .env files into process.env before parsing
@@ -27,6 +27,17 @@ const env = parseEnvironmentValues(process.env);
2727
const logger = createLogger(env);
2828
logger.info("Parsed environment values: %o", env);
2929

30+
let serverConfig;
31+
try {
32+
serverConfig = resolveServerConfig(env);
33+
} catch (error) {
34+
if (error instanceof ServerConfigError) {
35+
logger.fatal(error.message);
36+
process.exit(1);
37+
}
38+
throw error;
39+
}
40+
3041
const {
3142
port,
3243
baseUrl,
@@ -35,7 +46,7 @@ const {
3546
staticFilesVirtualPath,
3647
staticFilesPath,
3748
useHttps,
38-
} = resolveServerConfig(env);
49+
} = serverConfig;
3950

4051
const app = createApp({ configPath, staticFilesVirtualPath, staticFilesPath });
4152

0 commit comments

Comments
 (0)