Add lockfile binary and internal lib for interfacing with it

The linked issue describes the Supervisor not cleaning up locks it creates due
to crashing at just the wrong time. After internal discussion we decided to
differentiate Supervisor-created lockfiles from user-created lockfiles by using
the `nobody` UID (65534) for Supervisor-created lockfiles.

As the existing NPM lockfile lib does not allow creating lockfiles atomically
with different UIDs, we move to using the lockfile binary, which is part of the
procmail package. To allow nonroot users to write to lock directories, permissions
are changed to allow write access by nonroot users.

See: https://www.flowdock.com/app/rulemotion/r-resinos/threads/gWMgK5hmR26TzWGHux62NpgJtVl
Change-type: minor
Closes: #1758
Signed-off-by: Christina Wang <christina@balena.io>

Dockerfile.template

@@ -34,7 +34,9 @@ RUN apk add --no-cache \
 	sqlite-libs \
 	sqlite-dev \
 	dmidecode \
-	dbus-dev
+	dbus-dev \
+	procmail
+# procmail is installed for the lockfile binary
 
 COPY build-utils/node-sums.txt .
 
@@ -106,6 +108,7 @@ RUN apk add --no-cache \
 WORKDIR /usr/src/app
 
 COPY --from=BUILD /usr/local/bin/node /usr/local/bin/node
+COPY --from=BUILD /usr/bin/lockfile /usr/bin/lockfile
 COPY --from=BUILD /usr/src/app/dist ./dist
 COPY --from=BUILD /usr/src/app/package.json ./
 COPY --from=BUILD /usr/src/app/node_modules ./node_modules

entry.sh

@@ -56,6 +56,13 @@ fi
 # not a problem.
 modprobe ip6_tables || true
 
+export BASE_LOCK_DIR="/tmp/balena-supervisor/services"
+export LOCKFILE_UID=65534
+
+# Cleanup leftover Supervisor-created lockfiles from any previous processes.
+# Supervisor-created lockfiles have a UID of 65534.
+find "/mnt/root${BASE_LOCK_DIR}" -type f -user "${LOCKFILE_UID}" -name "*updates.lock" -delete
+
 if [ "${LIVEPUSH}" = "1" ]; then
 	exec npx nodemon --watch src --watch typings --ignore tests -e js,ts,json \
 		 --exec node -r ts-node/register/transpile-only src/app.ts

src/lib/lockfile.ts

@@ -0,0 +1,117 @@
+import * as fs from 'fs';
+import { dirname } from 'path';
+import { isRight } from 'fp-ts/lib/Either';
+
+import { exec, unlinkAll } from './fs-utils';
+import { NumericIdentifier } from '../types';
+
+// Equivalent to `drwxrwxrwt`
+const STICKY_WRITE_PERMISSIONS = 0o1777;
+export const BASE_LOCK_DIR =
+	process.env.BASE_LOCK_DIR || '/tmp/balena-supervisor/services';
+
+const decodedUid = NumericIdentifier.decode(process.env.LOCKFILE_UID);
+export const LOCKFILE_UID = isRight(decodedUid) ? decodedUid.right : 65534;
+
+/**
+ * Internal lockfile manager to track files in memory
+ */
+// Track locksTaken, so that the proper locks can be cleaned up on process exit
+const locksTaken: { [lockName: string]: boolean } = {};
+
+// Returns all current locks taken, as they've been stored in-memory.
+export const getLocksTaken = (): string[] => Object.keys(locksTaken);
+
+// Try to clean up any existing locks when the process exits
+process.on('exit', () => {
+	for (const lockName of getLocksTaken()) {
+		try {
+			unlockSync(lockName);
+		} catch (e) {
+			// Ignore unlocking errors
+		}
+	}
+});
+
+interface ChildProcessError {
+	code: number;
+	stderr: string;
+	stdout: string;
+}
+
+export class LockfileExistsError implements ChildProcessError {
+	public code: number;
+	public stderr: string;
+	public stdout: string;
+
+	constructor(path: string) {
+		this.code = 73;
+		this.stderr = `lockfile: Sorry, giving up on "${path}"`;
+		this.stdout = '';
+	}
+
+	// Check if an error is an instance of LockfileExistsError.
+	// This is necessary because the error thrown is a child process
+	// error that isn't typed by default, so instanceof will not work.
+	public static is(error: unknown): error is LockfileExistsError {
+		return (error as LockfileExistsError).code === 73;
+	}
+}
+
+export async function lock(path: string, uid = LOCKFILE_UID) {
+	/**
+	 * Set parent directory permissions to `drwxrwxrwt` (octal 1777), which are needed
+	 * for lockfile binary to run successfully as the any non-root uid, if executing
+	 * this command as a privileged uid.
+	 * NOTE: This will change the permissions of the parent directory at `path`,
+	 * which may not be expected if using lockfile as an independent module.
+	 *
+	 * `chmod` does not fail or throw if the directory already has the proper permissions.
+	 */
+	await fs.promises.chmod(dirname(path), STICKY_WRITE_PERMISSIONS);
+
+	/**
+	 * Run the lockfile binary as the provided UID. See https://linux.die.net/man/1/lockfile
+	 * `-r 0` means that lockfile will not retry if the lock exists.
+	 * If `uid` is not privileged or does not have write permissions to the path, this command will not succeed.
+	 */
+	try {
+		// Lock the file using binary
+		await exec(`lockfile -r 0 ${path}`, { uid });
+		// Store a lock in memory as taken
+		locksTaken[path] = true;
+	} catch (error) {
+		// Code 73 refers to EX_CANTCREAT (73) in sysexits.h, or:
+		// A (user specified) output file cannot be created.
+		// See: https://nxmnpg.lemoda.net/3/sysexits
+		if (LockfileExistsError.is(error)) {
+			// If error code is 73, updates.lock file already exists, so throw this error directly
+			throw error;
+		} else {
+			/**
+			 * For the most part, a child process error with code 73 should be thrown,
+			 * indicating the lockfile already exists. Any other error's child process
+			 * code should be included in the error message to more clearly signal
+			 * what went wrong. Other errors that are not the typical "file exists"
+			 * errors include but aren't limited to:
+			 *   - running out of file descriptors
+			 *   - binary corruption
+			 *   - other systems-based errors
+			 */
+			throw new Error(
+				`Got code ${(error as ChildProcessError).code} while locking updates: ${
+					(error as ChildProcessError).stderr
+				}`,
+			);
+		}
+	}
+}
+
+export async function unlock(path: string) {
+	// Removing the updates.lock file releases the lock
+	return await unlinkAll(path);
+}
+
+export function unlockSync(path: string) {
+	return fs.unlinkSync(path);
+}

test/src/lib/lockfile.spec.ts

@@ -0,0 +1,167 @@
+import { expect } from 'chai';
+import { dirname, basename } from 'path';
+import { stub, SinonStub } from 'sinon';
+import { promises as fs } from 'fs';
+import mock = require('mock-fs');
+
+import * as lockfile from '../../../src/lib/lockfile';
+import * as fsUtils from '../../../src/lib/fs-utils';
+import { ChildProcessError } from '../../../src/lib/errors';
+
+describe('lib/lockfile', () => {
+	const lockPath = `${lockfile.BASE_LOCK_DIR}/1234567/updates.lock`;
+	// mock-fs expects an octal file mode, however, Node's fs.stat.mode returns a bit field:
+	//   - 16877 (Node) == octal 0755 (drwxr-xr-x)
+	//   - 17407 (Node) == octal 1777 (drwxrwxrwt)
+	const DEFAULT_PERMISSIONS = {
+		unix: 0o755,
+		node: 16877,
+	};
+	const STICKY_WRITE_PERMISSIONS = {
+		unix: 0o1777,
+		node: 17407,
+	};
+
+	const mockDir = (
+		mode: number = DEFAULT_PERMISSIONS.unix,
+		opts: { createLock: boolean } = { createLock: false },
+	) => {
+		const items: any = {};
+		if (opts.createLock) {
+			items[basename(lockPath)] = mock.file({ uid: lockfile.LOCKFILE_UID });
+		}
+
+		mock({
+			[dirname(lockPath)]: mock.directory({
+				mode,
+				items,
+			}),
+		});
+	};
+
+	const checkLockDirFiles = async (
+		path: string,
+		opts: { shouldExist: boolean } = { shouldExist: true },
+	) => {
+		const files = await fs.readdir(dirname(path));
+		if (opts.shouldExist) {
+			expect(files).to.include(basename(path));
+		} else {
+			expect(files).to.have.length(0);
+		}
+	};
+
+	let execStub: SinonStub;
+
+	beforeEach(() => {
+		// @ts-ignore
+		execStub = stub(fsUtils, 'exec').callsFake(async (command, opts) => {
+			// Sanity check for the command call
+			expect(command.trim().startsWith('lockfile')).to.be.true;
+
+			// Remove any `lockfile` command options to leave just the command and the target filepath
+			const [, targetPath] = command
+				.replace(/-v|-nnn|-r\s+\d+|-l\s+\d+|-s\s+\d+|-!|-ml|-mu/g, '')
+				.split(/\s+/);
+
+			// Emulate the lockfile binary exec call
+			await fsUtils.touch(targetPath);
+			await fs.chown(targetPath, opts!.uid!, 0);
+		});
+	});
+
+	afterEach(() => {
+		execStub.restore();
+		mock.restore();
+	});
+
+	it('should create a lockfile as the `nobody` user at target path', async () => {
+		// Mock directory with default permissions
+		mockDir();
+
+		await lockfile.lock(lockPath);
+
+		// Verify lockfile exists
+		await checkLockDirFiles(lockPath, { shouldExist: true });
+
+		// Verify lockfile UID
+		expect((await fs.stat(lockPath)).uid).to.equal(lockfile.LOCKFILE_UID);
+	});
+
+	it('should create a lockfile with the provided `uid` if specified', async () => {
+		// Mock directory with default permissions
+		mockDir();
+
+		await lockfile.lock(lockPath, 2);
+
+		// Verify lockfile exists
+		await checkLockDirFiles(lockPath, { shouldExist: true });
+
+		// Verify lockfile UID
+		expect((await fs.stat(lockPath)).uid).to.equal(2);
+	});
+
+	it('should not create a lockfile if `lock` throws', async () => {
+		// Mock directory with default permissions
+		mockDir();
+
+		// Override default exec stub declaration, as it normally emulates a lockfile call with
+		// no errors, but we want it to throw an error just for this unit test
+		execStub.restore();
+
+		const childProcessError = new Error() as ChildProcessError;
+		childProcessError.code = 73;
+		childProcessError.stderr = 'lockfile: Test error';
+
+		execStub = stub(fsUtils, 'exec').throws(childProcessError);
+
+		try {
+			await lockfile.lock(lockPath);
+			expect.fail('lockfile.lock should have thrown an error');
+		} catch (err) {
+			expect(err).to.exist;
+		}
+
+		// Verify lockfile does not exist
+		await checkLockDirFiles(lockPath, { shouldExist: false });
+	});
+
+	it('should asynchronously unlock a lockfile', async () => {
+		// Mock directory with sticky + write permissions and existing lockfile
+		mockDir(STICKY_WRITE_PERMISSIONS.unix, { createLock: true });
+
+		// Verify lockfile exists
+		await checkLockDirFiles(lockPath, { shouldExist: true });
+
+		await lockfile.unlock(lockPath);
+
+		// Verify lockfile removal
+		await checkLockDirFiles(lockPath, { shouldExist: false });
+	});
+
+	it('should not error on async unlock if lockfile does not exist', async () => {
+		// Mock directory with sticky + write permissions
+		mockDir(STICKY_WRITE_PERMISSIONS.unix, { createLock: false });
+
+		// Verify lockfile does not exist
+		await checkLockDirFiles(lockPath, { shouldExist: false });
+
+		try {
+			await lockfile.unlock(lockPath);
+		} catch (err) {
+			expect.fail((err as Error)?.message ?? err);
+		}
+	});
+
+	it('should synchronously unlock a lockfile', () => {
+		// Mock directory with sticky + write permissions
+		mockDir(STICKY_WRITE_PERMISSIONS.unix, { createLock: true });
+
+		lockfile.unlockSync(lockPath);
+
+		// Verify lockfile does not exist
+		return checkLockDirFiles(lockPath, { shouldExist: false }).catch((err) => {
+			expect.fail((err as Error)?.message ?? err);
+		});
+	});
+});