Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[fix]: UI upgrade now runs as the same user as the js-controller #2950

Merged
merged 6 commits into from
Oct 25, 2024
Merged
Show file tree
Hide file tree
Changes from 5 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
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@
## __WORK IN PROGRESS__
-->

## __WORK IN PROGRESS__ - Lucy
* (@foxriver76) the UI upgrade now runs as the same user as the js-controller

## 7.0.1 (2024-10-21) - Lucy
* (@foxriver76) fixed crash case on database migration
* (@foxriver76) fixed edge case crash cases if notifications are processed nearly simultaneously
Expand Down
50 changes: 48 additions & 2 deletions packages/controller/src/lib/upgradeManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,18 @@ import fs from 'fs-extra';
import type { Socket } from 'node:net';
import type { Duplex } from 'node:stream';
import url from 'node:url';
import process from 'node:process';

/** The upgrade arguments provided to the constructor of the UpgradeManager */
export interface UpgradeArguments {
/** Version of controller to upgrade too */
version: string;
/** Admin instance which triggered the upgrade */
adminInstance: number;
/** User id the process should run as */
uid: number;
/** Group id the process should run as */
gid: number;
}

interface Certificates {
Expand Down Expand Up @@ -63,6 +69,10 @@ class UpgradeManager {
private readonly adminInstance: number;
/** Desired controller version */
private readonly version: string;
/** Group id the process should run as */
private readonly gid: number;
/** User id the process should run as */
private readonly uid: number;
/** Response send by webserver */
private readonly response: ServerResponse = {
running: true,
Expand All @@ -85,6 +95,30 @@ class UpgradeManager {
this.adminInstance = args.adminInstance;
this.version = args.version;
this.logger = this.setupLogger();
this.gid = args.gid;
this.uid = args.uid;

this.applyUser();
}

/**
* To prevent commands (including npm) running as root, we apply the passed in gid and uid
*/
private applyUser(): void {
if (!process.setuid || !process.setgid) {
const errMessage = 'Cannot ensure user and group ids on this system, because no POSIX platform';
this.log(errMessage, true);
throw new Error(errMessage);
}

try {
process.setgid(this.gid);
process.setuid(this.uid);
} catch (e) {
const errMessage = `Could not ensure user and group ids on this system: ${e.message}`;
this.log(errMessage, true);
throw new Error(errMessage);
}
}

/**
Expand All @@ -103,6 +137,8 @@ class UpgradeManager {

const version = additionalArgs[0];
const adminInstance = parseInt(additionalArgs[1]);
const uid = parseInt(additionalArgs[2]);
const gid = parseInt(additionalArgs[3]);

const isValid = !!valid(version);

Expand All @@ -116,7 +152,17 @@ class UpgradeManager {
throw new Error('Please provide a valid admin instance');
}

return { version, adminInstance };
if (isNaN(uid)) {
UpgradeManager.printUsage();
throw new Error('Please provide a valid uid');
}

if (isNaN(gid)) {
UpgradeManager.printUsage();
throw new Error('Please provide a valid gid');
}

return { version, adminInstance, uid, gid };
}

/**
Expand Down Expand Up @@ -163,7 +209,7 @@ class UpgradeManager {
* Print how the module should be used
*/
static printUsage(): void {
console.info('Example usage: "node upgradeManager.js <version> <adminInstance>"');
console.info('Example usage: "node upgradeManager.js <version> <adminInstance> <uid> <gid>"');
}

/**
Expand Down
11 changes: 9 additions & 2 deletions packages/controller/src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2760,7 +2760,12 @@ async function processMessage(msg: ioBroker.SendableMessage): Promise<null | voi
const { version, adminInstance } = msg.message;

logger.info(`${hostLogPrefix} Controller will upgrade itself to version ${version}`);
await startUpgradeManager({ version, adminInstance });
await startUpgradeManager({
version,
adminInstance,
uid: process.getuid ? process.getuid() : 0,
gid: process.getgid ? process.getgid() : 0,
});

if (msg.callback) {
sendTo(msg.from, msg.command, { result: true }, msg.callback);
Expand Down Expand Up @@ -5848,7 +5853,7 @@ async function upgradeOsPackages(packages: UpgradePacket[]): Promise<void> {
* @param options Arguments passed to the UpgradeManager process
*/
async function startUpgradeManager(options: UpgradeArguments): Promise<void> {
const { version, adminInstance } = options;
const { version, adminInstance, uid, gid } = options;
const upgradeProcessPath = require.resolve('./lib/upgradeManager');
let upgradeProcess: cp.ChildProcess;

Expand All @@ -5864,6 +5869,8 @@ async function startUpgradeManager(options: UpgradeArguments): Promise<void> {
upgradeProcessPath,
version,
adminInstance.toString(),
uid.toString(),
gid.toString(),
],
{
detached: true,
Expand Down
Loading