Skip to content

Commit

Permalink
Set SHELL env var in launcher if unset to avoid issues in Code's shel…
Browse files Browse the repository at this point in the history 
…l detection (#284)

* Detect shell in launcher and set env var if unset

Before launching Code, if the SHELL environment variable is unset, set
it to something before launching:

- If SHELL is unset and /bin/bash is installed, use that
- Otherwise, fall back to /bin/sh as a 'safe' option

This is necessary to avoid internal shell-detection logic in Code, which
reads /etc/passwd when SHELL is not set. This is an issue when running
in e.g. OpenShift, where cri-o will add a user entry to /etc/passwd
containing /sbin/nologin as the login shell.

* Add test cases to cover setting SHELL env var in launcher

* Fixup typo in workspace file creation logs

* Check /etc/passwd in launcher and only set SHELL if needed

When launching Code, add an additional check to read /etc/passwd and
check if the user has a (non-/sbin/nologin) shell defined there. If
/etc/passwd contains any other shell (e.g. /bin/zsh, /bin/bash, etc.),
then let Code read that shell from /etc/passwd instead of overriding it
by setting the SHELL environment variable.

---------

Signed-off-by: Angel Misevski <amisevsk@redhat.com>
  • Loading branch information
@amisevsk
amisevsk authored Oct 17, 2023
1 parent e940dc5 commit b71a618
Show file tree
Hide file tree
Showing 3 changed files with 271 additions and 1 deletion.
2 changes: 1 addition & 1 deletion launcher/src/code-workspace.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export class CodeWorkspace {
*
*****************************************************************************************************************/
async generate(): Promise<void> {
console.log("# Generaing Workspace file...");
console.log("# Generating Workspace file...");

if (!env.PROJECTS_ROOT) {
console.log(" > env.PROJECTS_ROOT is not set, skip this step");
Expand Down
25 changes: 25 additions & 0 deletions launcher/src/vscode-launcher.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
* SPDX-License-Identifier: EPL-2.0
***********************************************************************/

import { userInfo } from "os";
import { env } from "process";
import * as fs from "./fs-extra";

Expand Down Expand Up @@ -66,6 +67,17 @@ export class VSCodeLauncher {
env.NODE_EXTRA_CA_CERTS = NODE_EXTRA_CERTIFICATE;
}

if (!env.SHELL && userInfo().shell === "/sbin/nologin") {
// The SHELL env var is not set. In this case, Code will attempt to read the appropriate shell from /etc/passwd,
// which can cause issues when cri-o injects /sbin/nologin when starting containers. Instead, we'll check if bash
// is installed, and use that.
const shell = this.detectShell();
console.log(
` > SHELL environment variable is not set. Setting it to ${shell}`
);
env.SHELL = shell;
}

console.log(` > Running: ${node}`);
console.log(` > Params: ${params}`);

Expand All @@ -83,4 +95,17 @@ export class VSCodeLauncher {
console.log(`VS Code process exited with code ${code}`);
});
}

detectShell(): string {
try {
// Check if bash is installed
child_process.execSync("command -v /bin/bash", {
timeout: 500,
});
return "/bin/bash";
} catch (error) {
// bash not installed, fallback blindly to sh since it's at least better than /sbin/nologin
return "/bin/sh";
}
}
}
245 changes: 245 additions & 0 deletions launcher/tests/vscode-launcher.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
***********************************************************************/

import { env } from "process";
import * as os from "os";

import * as fs from "../src/fs-extra";
import * as child_process from "child_process";
Expand All @@ -22,6 +23,11 @@ describe("Test VS Code launcher:", () => {
delete env.PROJECT_SOURCE;
delete env.VSCODE_DEFAULT_WORKSPACE;
delete env.NODE_EXTRA_CA_CERTS;
delete env.SHELL;
});

afterEach(() => {
jest.restoreAllMocks();
});

test("should fail if env.VSCODE_NODEJS_RUNTIME_DIR is not set", async () => {
Expand Down Expand Up @@ -78,6 +84,7 @@ describe("Test VS Code launcher:", () => {
env.VSCODE_NODEJS_RUNTIME_DIR = "/tmp/vscode-nodejs-runtime";
env.PROJECTS_ROOT = "/tmp/projects";
env.VSCODE_DEFAULT_WORKSPACE = "/tmp/.code-workspace";
env.SHELL = "/bin/testshell";

const pathExistsMock = jest.fn();
Object.assign(fs, {
Expand Down Expand Up @@ -137,6 +144,7 @@ describe("Test VS Code launcher:", () => {
test("should launch VS Code with /projects/.code-workspace workspace file and Node extra certificate", async () => {
env.VSCODE_NODEJS_RUNTIME_DIR = "/tmp/vscode-nodejs-runtime";
env.PROJECTS_ROOT = "/tmp/projects";
env.SHELL = "/bin/testshell";

const pathExistsMock = jest.fn();
Object.assign(fs, {
Expand Down Expand Up @@ -194,6 +202,7 @@ describe("Test VS Code launcher:", () => {

env.PROJECTS_ROOT = "/tmp/projects";
env.PROJECT_SOURCE = "/tmp/projects/sample-project";
env.SHELL = "/bin/testshell";

const pathExistsMock = jest.fn();
Object.assign(fs, {
Expand Down Expand Up @@ -272,4 +281,240 @@ describe("Test VS Code launcher:", () => {
expect(spawnMock).not.toBeCalled();
}
});

test("should use SHELL env var when launching Code if set", async () => {
env.VSCODE_NODEJS_RUNTIME_DIR = "/tmp/vscode-nodejs-runtime";
env.PROJECTS_ROOT = "/tmp/projects";
env.VSCODE_DEFAULT_WORKSPACE = "/tmp/.code-workspace";
env.SHELL = "/bin/testshell";

const pathExistsMock = jest.fn();
Object.assign(fs, {
pathExists: pathExistsMock,
});

pathExistsMock.mockImplementation(async (path) => {
return "/tmp/.code-workspace" === path;
});

const spawnMock = jest.fn();
Object.assign(child_process, {
spawn: spawnMock,
});

const mainTreadEventEmitter = jest.fn();
const stdoutEventEmitter = jest.fn();
const stderrEventEmitter = jest.fn();

spawnMock.mockImplementation(() => ({
on: mainTreadEventEmitter,
stdout: {
on: stdoutEventEmitter,
},
stderr: {
on: stderrEventEmitter,
},
}));

const launcher = new VSCodeLauncher();
await launcher.launch();

expect(pathExistsMock).toBeCalledTimes(2);
expect(pathExistsMock).toBeCalledWith("/tmp/.code-workspace");
expect(pathExistsMock).toBeCalledWith(
"/tmp/node-extra-certificates/ca.crt"
);

expect(spawnMock).toBeCalledWith("/tmp/vscode-nodejs-runtime/node", [
"out/server-main.js",
"--host",
"127.0.0.1",
"--port",
"3100",
"--without-connection-token",
"--default-workspace",
"/tmp/.code-workspace",
]);

expect(env.NODE_EXTRA_CA_CERTS).not.toBeDefined();

expect(mainTreadEventEmitter).toBeCalledWith("close", expect.any(Function));
expect(stdoutEventEmitter).toBeCalledWith("data", expect.any(Function));
expect(stderrEventEmitter).toBeCalledWith("data", expect.any(Function));
});

test("should set SHELL env var to /bin/bash if unset and bash is installed", async () => {
env.VSCODE_NODEJS_RUNTIME_DIR = "/tmp/vscode-nodejs-runtime";
env.PROJECTS_ROOT = "/tmp/projects";
env.VSCODE_DEFAULT_WORKSPACE = "/tmp/.code-workspace";

jest
.spyOn(os, "userInfo")
.mockImplementation(() => ({ shell: "/sbin/nologin" } as any));

const pathExistsMock = jest.fn();
Object.assign(fs, {
pathExists: pathExistsMock,
});

pathExistsMock.mockImplementation(async (path) => {
return "/tmp/.code-workspace" === path;
});

const execSyncMock = jest.fn(() => "");

const spawnMock = jest.fn();
Object.assign(child_process, {
spawn: spawnMock,
execSync: execSyncMock,
});

const mainTreadEventEmitter = jest.fn();
const stdoutEventEmitter = jest.fn();
const stderrEventEmitter = jest.fn();

spawnMock.mockImplementation(() => ({
on: mainTreadEventEmitter,
stdout: {
on: stdoutEventEmitter,
},
stderr: {
on: stderrEventEmitter,
},
}));

const launcher = new VSCodeLauncher();
await launcher.launch();

expect(spawnMock).toBeCalledWith("/tmp/vscode-nodejs-runtime/node", [
"out/server-main.js",
"--host",
"127.0.0.1",
"--port",
"3100",
"--without-connection-token",
"--default-workspace",
"/tmp/.code-workspace",
]);

expect(env.SHELL).toEqual("/bin/bash");
});

test("should set SHELL env var to /bin/sh if unset and bash is not installed", async () => {
env.VSCODE_NODEJS_RUNTIME_DIR = "/tmp/vscode-nodejs-runtime";
env.PROJECTS_ROOT = "/tmp/projects";
env.VSCODE_DEFAULT_WORKSPACE = "/tmp/.code-workspace";

jest
.spyOn(os, "userInfo")
.mockImplementation(() => ({ shell: "/sbin/nologin" } as any));

const pathExistsMock = jest.fn();
Object.assign(fs, {
pathExists: pathExistsMock,
});

pathExistsMock.mockImplementation(async (path) => {
return "/tmp/.code-workspace" === path;
});

const execSyncMock = jest.fn(() => {
throw Error("test error");
});

const spawnMock = jest.fn();
Object.assign(child_process, {
spawn: spawnMock,
execSync: execSyncMock,
});

const mainTreadEventEmitter = jest.fn();
const stdoutEventEmitter = jest.fn();
const stderrEventEmitter = jest.fn();

spawnMock.mockImplementation(() => ({
on: mainTreadEventEmitter,
stdout: {
on: stdoutEventEmitter,
},
stderr: {
on: stderrEventEmitter,
},
}));

const launcher = new VSCodeLauncher();
await launcher.launch();

expect(spawnMock).toBeCalledWith("/tmp/vscode-nodejs-runtime/node", [
"out/server-main.js",
"--host",
"127.0.0.1",
"--port",
"3100",
"--without-connection-token",
"--default-workspace",
"/tmp/.code-workspace",
]);

expect(env.SHELL).toEqual("/bin/sh");
});

test("should not set SHELL env var if unset and /etc/passwd has a shell", async () => {
env.VSCODE_NODEJS_RUNTIME_DIR = "/tmp/vscode-nodejs-runtime";
env.PROJECTS_ROOT = "/tmp/projects";
env.VSCODE_DEFAULT_WORKSPACE = "/tmp/.code-workspace";

jest
.spyOn(os, "userInfo")
.mockImplementation(() => ({ shell: "/bin/zsh" } as any));

const pathExistsMock = jest.fn();
Object.assign(fs, {
pathExists: pathExistsMock,
});

pathExistsMock.mockImplementation(async (path) => {
return "/tmp/.code-workspace" === path;
});

const execSyncMock = jest.fn(() => {
throw Error("test error");
});

const spawnMock = jest.fn();
Object.assign(child_process, {
spawn: spawnMock,
execSync: execSyncMock,
});

const mainTreadEventEmitter = jest.fn();
const stdoutEventEmitter = jest.fn();
const stderrEventEmitter = jest.fn();

spawnMock.mockImplementation(() => ({
on: mainTreadEventEmitter,
stdout: {
on: stdoutEventEmitter,
},
stderr: {
on: stderrEventEmitter,
},
}));

const launcher = new VSCodeLauncher();
await launcher.launch();

expect(spawnMock).toBeCalledWith("/tmp/vscode-nodejs-runtime/node", [
"out/server-main.js",
"--host",
"127.0.0.1",
"--port",
"3100",
"--without-connection-token",
"--default-workspace",
"/tmp/.code-workspace",
]);

expect(env.SHELL).toBe(undefined);
});
});

0 comments on commit b71a618

Please sign in to comment.