Skip to content

Commit a3a4e10

Browse files
committed
fix: harden supervisor log permissions
1 parent 47a3143 commit a3a4e10

4 files changed

Lines changed: 104 additions & 3 deletions

File tree

src/platform/paths.js

Lines changed: 68 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
// @ts-check
22

3-
import { chmod, lstat, mkdir } from 'node:fs/promises';
3+
import { chmod, lstat, mkdir, open } from 'node:fs/promises';
44
import { homedir } from 'node:os';
55
import { dirname, join, resolve } from 'node:path';
66
import { OWNERSHIP_MARKER_FILENAME, ensureOwnershipMarker } from './ownership.js';
@@ -36,7 +36,12 @@ export function resolveRuntimePaths(environment = process.env) {
3636
}
3737

3838
/**
39-
* @param {{applicationDirectory: string, socketPath: string}} paths
39+
* @param {{
40+
* applicationDirectory: string,
41+
* socketPath: string,
42+
* supervisorStandardOutputPath?: string,
43+
* supervisorStandardErrorPath?: string
44+
* }} paths
4045
*/
4146
export async function prepareRuntimeDirectories(paths) {
4247
await ensurePrivateDirectory(paths.applicationDirectory);
@@ -48,6 +53,14 @@ export async function prepareRuntimeDirectories(paths) {
4853
if (socketDirectory !== paths.applicationDirectory) {
4954
await ensurePrivateDirectory(socketDirectory);
5055
}
56+
for (const path of [
57+
paths.supervisorStandardOutputPath,
58+
paths.supervisorStandardErrorPath,
59+
]) {
60+
if (path !== undefined) {
61+
await ensurePrivateFile(path);
62+
}
63+
}
5164
}
5265

5366
/**
@@ -110,13 +123,65 @@ async function ensurePrivateDirectory(directory) {
110123
}
111124
}
112125

126+
/**
127+
* Pre-create supervisor logs so native supervisors cannot create them with a
128+
* permissive inherited umask or follow an existing symbolic link.
129+
*
130+
* @param {string} path
131+
*/
132+
async function ensurePrivateFile(path) {
133+
let information;
134+
try {
135+
information = await lstat(path);
136+
} catch (error) {
137+
if (!isMissingFile(error)) {
138+
throw error;
139+
}
140+
let handle;
141+
try {
142+
handle = await open(path, 'wx', 0o600);
143+
await handle.chmod(0o600);
144+
information = await handle.stat();
145+
} catch (creationError) {
146+
if (!hasCode(creationError, 'EEXIST')) {
147+
throw creationError;
148+
}
149+
information = await lstat(path);
150+
} finally {
151+
await handle?.close();
152+
}
153+
}
154+
if (!information.isFile() || information.isSymbolicLink()) {
155+
throw new Error(`Unsafe Portreeve runtime file: ${path}`);
156+
}
157+
if (typeof process.getuid === 'function' && information.uid !== process.getuid()) {
158+
throw new Error(`Portreeve runtime file has another owner: ${path}`);
159+
}
160+
if ((information.mode & 0o077) !== 0) {
161+
throw new Error(
162+
`Portreeve runtime file is not private: ${path} (mode ${(information.mode & 0o777).toString(8)})`,
163+
);
164+
}
165+
if ((information.mode & 0o600) !== 0o600) {
166+
throw new Error(`Portreeve runtime file lacks owner access: ${path}`);
167+
}
168+
}
169+
113170
/**
114171
* @param {unknown} error
115172
*/
116173
function isMissingFile(error) {
174+
return hasCode(error, 'ENOENT');
175+
}
176+
177+
/**
178+
* @param {unknown} error
179+
* @param {string} code
180+
*/
181+
function hasCode(error, code) {
117182
return (
118183
error instanceof Error &&
119184
'code' in error &&
120-
/** @type {{code?: string}} */ (error).code === 'ENOENT'
185+
/** @type {{code?: string}} */ (error).code === code
121186
);
122187
}

src/supervision/systemd.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ Description=Portreeve local development port authority
4141
[Service]
4242
Type=simple
4343
Environment=PORTREEVE_SUPERVISED=1
44+
UMask=0077
4445
ExecStart=${command}
4546
Restart=on-failure
4647
StandardOutput=append:${escapeSystemdSpecifiers(definition.standardOutputPath)}

test/platform/paths.test.js

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,14 +30,48 @@ test('creates private application and socket directories', async () => {
3030
const root = await directory();
3131
const applicationDirectory = join(root, 'data');
3232
const socketDirectory = join(root, 'runtime');
33+
const supervisorStandardOutputPath = join(
34+
applicationDirectory,
35+
'supervisor.stdout.log',
36+
);
37+
const supervisorStandardErrorPath = join(
38+
applicationDirectory,
39+
'supervisor.stderr.log',
40+
);
3341

3442
await prepareRuntimeDirectories({
3543
applicationDirectory,
3644
socketPath: join(socketDirectory, 'portreeve.sock'),
45+
supervisorStandardOutputPath,
46+
supervisorStandardErrorPath,
3747
});
3848

3949
expect((await stat(applicationDirectory)).mode & 0o777).toBe(0o700);
4050
expect((await stat(socketDirectory)).mode & 0o777).toBe(0o700);
51+
expect((await stat(supervisorStandardOutputPath)).mode & 0o777).toBe(0o600);
52+
expect((await stat(supervisorStandardErrorPath)).mode & 0o777).toBe(0o600);
53+
});
54+
55+
test('rejects unsafe existing supervisor logs rather than repairing them', async () => {
56+
const root = await directory();
57+
const applicationDirectory = join(root, 'data');
58+
const supervisorStandardOutputPath = join(
59+
applicationDirectory,
60+
'supervisor.stdout.log',
61+
);
62+
await mkdir(applicationDirectory);
63+
await chmod(applicationDirectory, 0o700);
64+
await writeFile(supervisorStandardOutputPath, '');
65+
await chmod(supervisorStandardOutputPath, 0o664);
66+
67+
await expect(
68+
prepareRuntimeDirectories({
69+
applicationDirectory,
70+
socketPath: join(applicationDirectory, 'portreeve.sock'),
71+
supervisorStandardOutputPath,
72+
}),
73+
).rejects.toThrow(/not private|writable by another user/u);
74+
expect((await stat(supervisorStandardOutputPath)).mode & 0o777).toBe(0o664);
4175
});
4276

4377
test('rejects rather than repairs an unsafe existing directory', async () => {

test/supervision/native-adapters.test.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,7 @@ describe('native supervisor adapters', () => {
119119
try {
120120
const content = supervisor.renderDefinition(definition);
121121
expect(content).toContain('Environment=PORTREEVE_SUPERVISED=1');
122+
expect(content).toContain('UMask=0077');
122123
expect(content).toContain('%%');
123124
expect(content).toContain('"/Users/Example User/.portreeve/bin/portreeve"');
124125
await supervisor.installDefinition(content);

0 commit comments

Comments
 (0)