From c67f5c8873dbd75864a098301c9a8db35383082c Mon Sep 17 00:00:00 2001
From: Christof Marti <chrmarti@microsoft.com>
Date: Fri, 1 Jul 2022 13:59:03 +0200
Subject: [PATCH] Use previous USER (devcontainers/cli#70)

---
 src/spec-node/containerFeatures.ts |  8 +++----
 src/spec-node/dockerCompose.ts     | 11 +++++----
 src/spec-node/singleContainer.ts   |  5 ++--
 src/spec-node/utils.ts             | 33 +++++++++++++++++++++++---
 src/test/dockerfileUtils.test.ts   | 38 ++++++++++++++++++++++++++++--
 5 files changed, 79 insertions(+), 16 deletions(-)

diff --git a/src/spec-node/containerFeatures.ts b/src/spec-node/containerFeatures.ts
index d1d53a211..28a55377a 100644
--- a/src/spec-node/containerFeatures.ts
+++ b/src/spec-node/containerFeatures.ts
@@ -40,7 +40,7 @@ export async function extendImage(params: DockerResolverParameters, config: DevC
 		};
 	};
 
-	const imageUser = (await imageDetails()).Config.User || 'root';
+	const imageUser = async () => (await imageDetails()).Config.User || 'root';
 	const extendImageDetails = await getExtendImageBuildInfo(params, config, imageName, imageUser, imageLabelDetails);
 	if (!extendImageDetails || !extendImageDetails.featureBuildInfo) {
 		// no feature extensions - return
@@ -96,7 +96,7 @@ export async function extendImage(params: DockerResolverParameters, config: DevC
 	return { updatedImageName, collapsedFeaturesConfig, imageDetails };
 }
 
-export async function getExtendImageBuildInfo(params: DockerResolverParameters, config: DevContainerConfig, baseName: string, imageUser: string, imageLabelDetails: () => Promise<{ definition: string | undefined; version: string | undefined }>) {
+export async function getExtendImageBuildInfo(params: DockerResolverParameters, config: DevContainerConfig, baseName: string, imageUser: () => Promise<string>, imageLabelDetails: () => Promise<{ definition: string | undefined; version: string | undefined }>) {
 
 	const featuresConfig = await generateFeaturesConfig(params.common, (await createFeaturesTempFolder(params.common)), config, imageLabelDetails, getContainerFeaturesFolder);
 	if (!featuresConfig) {
@@ -125,7 +125,7 @@ export function generateContainerEnvs(featuresConfig: FeaturesConfig) {
 	return result;
 }
 
-async function getContainerFeaturesBuildInfo(params: DockerResolverParameters, featuresConfig: FeaturesConfig, baseName: string, imageUser: string): Promise<{ dstFolder: string; dockerfileContent: string; dockerfilePrefixContent: string; buildArgs: Record<string, string>; buildKitContexts: Record<string, string> } | null> {
+async function getContainerFeaturesBuildInfo(params: DockerResolverParameters, featuresConfig: FeaturesConfig, baseName: string, imageUser: () => Promise<string>): Promise<{ dstFolder: string; dockerfileContent: string; dockerfilePrefixContent: string; buildArgs: Record<string, string>; buildKitContexts: Record<string, string> } | null> {
 	const { common } = params;
 	const { cliHost, output } = common;
 	const { dstFolder } = featuresConfig;
@@ -264,7 +264,7 @@ ARG _DEV_CONTAINERS_BASE_IMAGE=placeholder
 		dockerfilePrefixContent,
 		buildArgs: {
 			_DEV_CONTAINERS_BASE_IMAGE: baseName,
-			_DEV_CONTAINERS_IMAGE_USER: imageUser,
+			_DEV_CONTAINERS_IMAGE_USER: await imageUser(),
 			_DEV_CONTAINERS_FEATURE_CONTENT_SOURCE: buildContentImageName,
 		},
 		buildKitContexts: useBuildKitBuildContexts ? { dev_containers_feature_content_source: dstFolder } : {},
diff --git a/src/spec-node/dockerCompose.ts b/src/spec-node/dockerCompose.ts
index 2bf87d6ec..693577e03 100644
--- a/src/spec-node/dockerCompose.ts
+++ b/src/spec-node/dockerCompose.ts
@@ -6,7 +6,7 @@
 import * as yaml from 'js-yaml';
 import * as shellQuote from 'shell-quote';
 
-import { createContainerProperties, startEventSeen, ResolverResult, getTunnelInformation, DockerResolverParameters, inspectDockerImage, ensureDockerfileHasFinalStageName } from './utils';
+import { createContainerProperties, startEventSeen, ResolverResult, getTunnelInformation, DockerResolverParameters, inspectDockerImage, ensureDockerfileHasFinalStageName, getImageUser } from './utils';
 import { ContainerProperties, setupInContainer, ResolverProgress } from '../spec-common/injectHeadless';
 import { ContainerError } from '../spec-common/errors';
 import { Workspace } from '../spec-utils/workspaces';
@@ -150,12 +150,13 @@ export async function buildAndExtendDockerCompose(config: DevContainerFromDocker
 
 	// determine base imageName for generated features build stage(s)
 	let baseName = 'dev_container_auto_added_stage_label';
-	let dockerfile = null;
+	let dockerfile: string;
+	let originalDockerfile: string;
 	const serviceInfo = getBuildInfoForService(composeService, cliHost.path, localComposeFiles);
 	if (serviceInfo.build) {
 		const { context, dockerfilePath, target } = serviceInfo.build;
 		const resolvedDockerfilePath = cliHost.path.isAbsolute(dockerfilePath) ? dockerfilePath : path.resolve(context, dockerfilePath);
-		dockerfile = (await cliHost.readFile(resolvedDockerfilePath)).toString();
+		dockerfile = originalDockerfile = (await cliHost.readFile(resolvedDockerfilePath)).toString();
 		if (target) {
 			// Explictly set build target for the dev container build features on that
 			baseName = target;
@@ -169,13 +170,13 @@ export async function buildAndExtendDockerCompose(config: DevContainerFromDocker
 			}
 		}
 	} else {
-		dockerfile = `FROM ${composeService.image} AS ${baseName}\n`;
+		dockerfile = originalDockerfile = `FROM ${composeService.image} AS ${baseName}\n`;
 	}
 
 	// determine whether we need to extend with features
 	const labelDetails = async () => { return { definition: undefined, version: undefined }; };
 	const noBuildKitParams = { ...params, buildKitVersion: null }; // skip BuildKit -> can't set additional build contexts with compose
-	const extendImageBuildInfo = await getExtendImageBuildInfo(noBuildKitParams, config, baseName, config.remoteUser ?? 'root', labelDetails);
+	const extendImageBuildInfo = await getExtendImageBuildInfo(noBuildKitParams, config, baseName, () => getImageUser(params, originalDockerfile), labelDetails);
 
 	let buildOverrideContent = null;
 	if (extendImageBuildInfo) {
diff --git a/src/spec-node/singleContainer.ts b/src/spec-node/singleContainer.ts
index 1c0e6477b..1b60256cc 100644
--- a/src/spec-node/singleContainer.ts
+++ b/src/spec-node/singleContainer.ts
@@ -4,7 +4,7 @@
  *--------------------------------------------------------------------------------------------*/
 
 
-import { createContainerProperties, startEventSeen, ResolverResult, getTunnelInformation, getDockerfilePath, getDockerContextPath, DockerResolverParameters, isDockerFileConfig, uriToWSLFsPath, WorkspaceConfiguration, getFolderImageName, inspectDockerImage, ensureDockerfileHasFinalStageName } from './utils';
+import { createContainerProperties, startEventSeen, ResolverResult, getTunnelInformation, getDockerfilePath, getDockerContextPath, DockerResolverParameters, isDockerFileConfig, uriToWSLFsPath, WorkspaceConfiguration, getFolderImageName, inspectDockerImage, ensureDockerfileHasFinalStageName, getImageUser } from './utils';
 import { ContainerProperties, setupInContainer, ResolverProgress } from '../spec-common/injectHeadless';
 import { ContainerError, toErrorText } from '../spec-common/errors';
 import { ContainerDetails, listContainers, DockerCLIParameters, inspectContainers, dockerCLI, dockerPtyCLI, toPtyExecParameters, ImageDetails, toExecParameters } from '../spec-shutdown/dockerUtils';
@@ -122,6 +122,7 @@ async function buildAndExtendImage(buildParams: DockerResolverParameters, config
 	}
 
 	let dockerfile = (await cliHost.readFile(dockerfilePath)).toString();
+	const originalDockerfile = dockerfile;
 	let baseName = 'dev_container_auto_added_stage_label';
 	if (config.build?.target) {
 		// Explictly set build target for the dev container build features on that
@@ -137,7 +138,7 @@ async function buildAndExtendImage(buildParams: DockerResolverParameters, config
 	}
 
 	const labelDetails = async () => { return { definition: undefined, version: undefined }; };
-	const extendImageBuildInfo = await getExtendImageBuildInfo(buildParams, config, baseName, config.remoteUser ?? 'root', labelDetails);
+	const extendImageBuildInfo = await getExtendImageBuildInfo(buildParams, config, baseName, () => getImageUser(buildParams, originalDockerfile), labelDetails);
 
 	let finalDockerfilePath = dockerfilePath;
 	const additionalBuildArgs: string[] = [];
diff --git a/src/spec-node/utils.ts b/src/spec-node/utils.ts
index b33ce58c7..f19f85cfd 100644
--- a/src/spec-node/utils.ts
+++ b/src/spec-node/utils.ts
@@ -15,7 +15,7 @@ import { ContainerProperties, getContainerProperties, ResolverParameters } from
 import { Workspace } from '../spec-utils/workspaces';
 import { URI } from 'vscode-uri';
 import { ShellServer } from '../spec-common/shellServer';
-import { inspectContainer, inspectImage, getEvents, ContainerDetails, DockerCLIParameters, dockerExecFunction, dockerPtyCLI, dockerPtyExecFunction, toDockerImageName, DockerComposeCLI } from '../spec-shutdown/dockerUtils';
+import { inspectContainer, inspectImage, getEvents, ContainerDetails, DockerCLIParameters, dockerExecFunction, dockerPtyCLI, dockerPtyExecFunction, toDockerImageName, DockerComposeCLI, ImageDetails } from '../spec-shutdown/dockerUtils';
 import { getRemoteWorkspaceFolder } from './dockerCompose';
 import { findGitRootFolder } from '../spec-common/git';
 import { parentURI, uriToFsPath } from '../spec-configuration/configurationCommonUtils';
@@ -332,17 +332,44 @@ export function getLocalCacheFolder(): string {
 	return path.join(os.tmpdir(), process.platform === 'linux' ? `vsch-${os.userInfo().username}` : 'vsch');
 }
 
+const findFromLines = new RegExp(/^(?<line>\s*FROM.*)/, 'gm');
+const parseFromLine = /FROM\s+(?<platform>--platform=\S+\s+)?(?<image>\S+)(\s+[Aa][Ss]\s+(?<label>[^\s]+))?/;
+const findUserLines = new RegExp(/^\s*USER\s+(?<user>\S+)/, 'gm');
+
+export async function getImageUser(params: DockerResolverParameters, dockerfile: string) {
+	return internalGetImageUser(imageName => inspectDockerImage(params, imageName, true), dockerfile);
+}
+
+export async function internalGetImageUser(inspectDockerImage: (imageName: string) => Promise<ImageDetails>, dockerfile: string) {
+	// TODO: Other targets.
+	const userLines = [...dockerfile.matchAll(findUserLines)];
+	if (userLines.length) {
+		const user = userLines[userLines.length - 1].groups?.user;
+		if (user && user.indexOf('$') === -1) { // Ignore variables.
+			return user;
+		}
+	}
+	const fromLine = [...dockerfile.matchAll(findFromLines)][0];
+	const fromMatch = fromLine?.groups?.line?.match(parseFromLine);
+	const imageName = fromMatch?.groups?.image;
+	if (!imageName) {
+		return 'root';
+	}
+	const imageDetails = await inspectDockerImage(imageName);
+	return imageDetails.Config.User || 'root';
+}
+
 // not expected to be called externally (exposed for testing)
 export function ensureDockerfileHasFinalStageName(dockerfile: string, defaultLastStageName: string): { lastStageName: string; modifiedDockerfile: string | undefined } {
 
 	// Find the last line that starts with "FROM" (possibly preceeded by white-space)
-	const fromLines = [...dockerfile.matchAll(new RegExp(/^(?<line>\s*FROM.*)/, 'gm'))];
+	const fromLines = [...dockerfile.matchAll(findFromLines)];
 	const lastFromLineMatch = fromLines[fromLines.length - 1];
 	const lastFromLine = lastFromLineMatch.groups?.line as string;
 
 	// Test for "FROM [--platform=someplat] base [as label]"
 	// That is, match against optional platform and label
-	const fromMatch = lastFromLine.match(/FROM\s+(?<platform>--platform=\S+\s+)?\S+(\s+[Aa][Ss]\s+(?<label>[^\s]+))?/);
+	const fromMatch = lastFromLine.match(parseFromLine);
 	if (!fromMatch) {
 		throw new Error('Error parsing Dockerfile: failed to parse final FROM line');
 	}
diff --git a/src/test/dockerfileUtils.test.ts b/src/test/dockerfileUtils.test.ts
index 6370db21b..ed033c652 100644
--- a/src/test/dockerfileUtils.test.ts
+++ b/src/test/dockerfileUtils.test.ts
@@ -1,5 +1,6 @@
 import { assert } from 'chai';
-import { ensureDockerfileHasFinalStageName } from '../spec-node/utils';
+import { ensureDockerfileHasFinalStageName, internalGetImageUser } from '../spec-node/utils';
+import { ImageDetails } from '../spec-shutdown/dockerUtils';
 
 describe('ensureDockerfileHasFinalStageName', () => {
 
@@ -139,4 +140,37 @@ RUN another command
             });
         });
     });
-});
\ No newline at end of file
+});
+
+describe('getImageUser', () => {
+
+    it('for a simple FROM line', async () => {
+        const dockerfile = `FROM debian:latest as base
+FROM ubuntu:latest as dev
+`;
+        const details: ImageDetails = {
+            Id: '123',
+            Config: {
+                User: 'imageUser',
+                Env: null,
+                Labels: null,
+                Entrypoint: null,
+                Cmd: null
+            }
+        };
+        assert.strictEqual(await internalGetImageUser(async imageName => {
+            assert.strictEqual(imageName, 'debian:latest');
+            return details;
+        }, dockerfile), 'imageUser');
+    });
+
+    it('for a USER', async () => {
+        const dockerfile = `FROM ubuntu:latest as base
+USER dockerfileUserA
+USER dockerfileUserB
+`;
+        assert.strictEqual(await internalGetImageUser(() => {
+            throw new Error('ooops');
+        }, dockerfile), 'dockerfileUserB');
+    });
+});