Skip to content

Commit a48d4b0

Browse files
committed
bug fixes
1 parent 3baffb4 commit a48d4b0

6 files changed

Lines changed: 105 additions & 5 deletions

File tree

.prettierignore

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
# Generated frontend bundles (built by esbuild) - reformatting these would
2+
# corrupt the minified output that `npm run build` produces.
3+
public/app.js
4+
public/login.js
5+
6+
# Generated CSS (compiled from LESS).
7+
public/styles.css
8+
9+
# Dependencies and build output.
10+
node_modules
11+
dist
12+
build

lib/config.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,10 @@ function envInt(name, fallback) {
2727
if (raw === undefined || raw === '') {
2828
return fallback;
2929
}
30-
return Number.parseInt(raw, 10);
30+
const parsed = Number.parseInt(raw, 10);
31+
// A non-numeric value (e.g. PORT=abc) would otherwise yield NaN and silently
32+
// misconfigure the app; fall back to the documented default instead.
33+
return Number.isNaN(parsed) ? fallback : parsed;
3134
}
3235

3336
/** Read an env var as a string with a fallback default. */

lib/service/logBus.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,8 +90,8 @@ export async function startLogBus() {
9090
}
9191
}
9292

93-
bus.on('log:out', (packet) => handlePacket(packet, false));
94-
bus.on('log:err', (packet) => handlePacket(packet, true));
93+
bus.on('log:out', handlePacket);
94+
bus.on('log:err', handlePacket);
9595

9696
bus.on('error', (err) => {
9797
logger.warn(`[LOG_BUS] Bus error: ${err?.message ?? err}`);

lib/transport/router.js

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -347,21 +347,27 @@ router.delete('/api/processes/:id', requireAuth, validateProcessId, async (req,
347347
);
348348
return res.status(403).json({ error: 'CSRF token mismatch' });
349349
}
350+
// Resolve the PM2 name *before* deleting: req.params.id may be a numeric
351+
// pm_id, but the monitoring and deployment records are keyed by name. Once
352+
// the process is gone from PM2 it can no longer be resolved, so cleanup keyed
353+
// off the numeric id would silently miss (the process would reappear as an
354+
// orphan and the deploy directory would never be removed).
355+
const pm2Name = (await resolvePm2Name(req.params.id).catch(() => null)) ?? req.params.id;
356+
350357
logger.info(
351358
`[ACTION] User: ${req.session.username} is deleting process: ${req.params.id} from identity: ${getClientIdentity(req)}`,
352359
);
353360
await pm2.deleteProcess(req.params.id);
354361
// Remove the monitoring record if one exists so the process no longer
355362
// appears as an orphan after deletion.
356363
try {
357-
removeMonitored(req.params.id);
364+
removeMonitored(pm2Name);
358365
} catch {
359366
// No monitoring record - nothing to clean up.
360367
}
361368

362369
// Optionally purge the deployment record and its directory on disk.
363370
if (req.query.deleteDeploy === 'true') {
364-
const pm2Name = req.params.id;
365371
const deployment = getDeploymentByName(pm2Name);
366372
if (deployment) {
367373
if (deployment.deploy_path) {
@@ -703,6 +709,17 @@ router.post('/api/settings/general', requireAuth, (req, res) => {
703709
return res.status(400).json({ error: '"settings" must be a plain object.' });
704710
}
705711

712+
// Values are written verbatim as KEY=VALUE lines. A line break in a value
713+
// would inject arbitrary additional env entries (e.g. overwriting
714+
// AUTH_PASSWORD_HASH), so reject any value containing CR or LF.
715+
const lineBreakKeys = Object.entries(settings)
716+
.filter(([key]) => key !== 'authPassword')
717+
.filter(([, value]) => value != null && /[\r\n]/.test(String(value)))
718+
.map(([key]) => key);
719+
if (lineBreakKeys.length > 0) {
720+
return res.status(400).json({ error: `Values must not contain line breaks: ${lineBreakKeys.join(', ')}` });
721+
}
722+
706723
try {
707724
const envPath = path.resolve(__dirname, '..', '..', '.env');
708725

lib/transport/ws.js

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,14 @@ function handleUnifiedStream(ws) {
135135
let detailInterval = null;
136136
let logUnsubscribe = null;
137137

138+
// Liveness flag for the server-level ping/pong loop (see attachWebSocketServer).
139+
// A half-open connection that vanished without a close frame never resets this,
140+
// so the loop can terminate it and free its intervals and log subscription.
141+
ws.isAlive = true;
142+
ws.on('pong', () => {
143+
ws.isAlive = true;
144+
});
145+
138146
send(ws, 'connected', {});
139147

140148
// Process list — always running.
@@ -282,6 +290,9 @@ export function broadcastDeployProgress(deploymentId, stage, line, status) {
282290

283291
const STREAM_PATH = '/ws/stream';
284292

293+
/** Interval between protocol-level ping frames used for dead-connection detection. */
294+
const HEARTBEAT_INTERVAL_MS = 30000;
295+
285296
/**
286297
* Attach the unified WebSocket server to an existing HTTP server.
287298
*
@@ -290,6 +301,25 @@ const STREAM_PATH = '/ws/stream';
290301
export function attachWebSocketServer(server) {
291302
_wss = new WebSocketServer({ noServer: true });
292303

304+
// Protocol-level ping/pong sweep: terminate connections that did not answer
305+
// the previous ping (half-open sockets from laptop sleep, NAT timeouts, or
306+
// dropped networks never fire 'close' on their own), then ping the rest.
307+
// Terminating fires 'close', which tears down the per-connection intervals
308+
// and log subscription in handleUnifiedStream.
309+
const heartbeat = setInterval(() => {
310+
for (const ws of _wss.clients) {
311+
if (ws.isAlive === false) {
312+
ws.terminate();
313+
continue;
314+
}
315+
ws.isAlive = false;
316+
ws.ping();
317+
}
318+
}, HEARTBEAT_INTERVAL_MS);
319+
heartbeat.unref?.();
320+
321+
_wss.on('close', () => clearInterval(heartbeat));
322+
293323
server.on('upgrade', (req, socket, head) => {
294324
const session = authenticate(req);
295325
if (!session) {

test/api.settings.test.js

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,44 @@ describe('POST /api/settings/general', () => {
234234
}
235235
}
236236
});
237+
238+
it('rejects values containing line breaks to prevent .env injection', async () => {
239+
const { cookie, csrfToken } = await getAuthSession();
240+
241+
const realEnvPath = path.resolve(__dirname, '..', '.env');
242+
let originalEnv = null;
243+
try {
244+
originalEnv = fs.readFileSync(realEnvPath, 'utf8');
245+
} catch {
246+
// .env may not exist in CI.
247+
}
248+
fs.writeFileSync(realEnvPath, 'HOST=0.0.0.0\nPORT=3030\n', 'utf8');
249+
250+
try {
251+
const res = await request(app)
252+
.post('/api/settings/general')
253+
.set('Cookie', cookie)
254+
.set('X-CSRF-Token', csrfToken)
255+
.send({ settings: { HOST: '0.0.0.0\nAUTH_PASSWORD_HASH=deadbeef' } });
256+
257+
expect(res.status).to.equal(400);
258+
expect(res.body.error).to.include('line breaks');
259+
260+
// The injected line must not have been written to disk.
261+
const written = fs.readFileSync(realEnvPath, 'utf8');
262+
expect(written).to.not.include('AUTH_PASSWORD_HASH=deadbeef');
263+
} finally {
264+
if (originalEnv !== null) {
265+
fs.writeFileSync(realEnvPath, originalEnv, 'utf8');
266+
} else {
267+
try {
268+
fs.unlinkSync(realEnvPath);
269+
} catch {
270+
/* ignore */
271+
}
272+
}
273+
}
274+
});
237275
});
238276

239277
// ── Notification prefs ────────────────────────────────────────────────────────

0 commit comments

Comments
 (0)