From 39028a332609f5704f694a470380f4ac28643e53 Mon Sep 17 00:00:00 2001 From: Gaurav Saini <147703805+gauravsaini04@users.noreply.github.com> Date: Wed, 6 Nov 2024 12:54:57 +0000 Subject: [PATCH 1/3] cli issue - Valid devcontainer.metadata can corrupt compose override file #904 --- src/spec-node/dockerCompose.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/spec-node/dockerCompose.ts b/src/spec-node/dockerCompose.ts index 89f810213..8f7891adc 100644 --- a/src/spec-node/dockerCompose.ts +++ b/src/spec-node/dockerCompose.ts @@ -557,7 +557,7 @@ while sleep 1 & wait $$!; do :; done", "-"${userEntrypoint.map(a => `, ${JSON.st init: true` : ''}${user ? ` user: ${user}` : ''}${Object.keys(env).length ? ` environment:${Object.keys(env).map(key => ` - - ${key}=${env[key]}`).join('')}` : ''}${mergedConfig.privileged ? ` + - "${key}=${env[key].replace(/"/g, '\\"')}"`).join('')}` : ''}${mergedConfig.privileged ? ` privileged: true` : ''}${capAdd.length ? ` cap_add:${capAdd.map(cap => ` - ${cap}`).join('')}` : ''}${securityOpts.length ? ` From ee1627cb3f648c2a4bf898695ced8663e9c6ed8c Mon Sep 17 00:00:00 2001 From: Kaniska244 Date: Wed, 27 Nov 2024 08:32:33 +0000 Subject: [PATCH 2/3] Change done to wrap the containerEnv properties with single quote --- src/spec-node/dockerCompose.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/spec-node/dockerCompose.ts b/src/spec-node/dockerCompose.ts index 8f7891adc..e36e6d817 100644 --- a/src/spec-node/dockerCompose.ts +++ b/src/spec-node/dockerCompose.ts @@ -557,7 +557,7 @@ while sleep 1 & wait $$!; do :; done", "-"${userEntrypoint.map(a => `, ${JSON.st init: true` : ''}${user ? ` user: ${user}` : ''}${Object.keys(env).length ? ` environment:${Object.keys(env).map(key => ` - - "${key}=${env[key].replace(/"/g, '\\"')}"`).join('')}` : ''}${mergedConfig.privileged ? ` + - '${key}=${env[key].replace(/'/g, '\'\'')}'`).join('')}` : ''}${mergedConfig.privileged ? ` privileged: true` : ''}${capAdd.length ? ` cap_add:${capAdd.map(cap => ` - ${cap}`).join('')}` : ''}${securityOpts.length ? ` From 7d9ca4899dcaee5dc59e9f4cfe0ab77fa95c11e1 Mon Sep 17 00:00:00 2001 From: Kaniska244 Date: Tue, 3 Dec 2024 12:36:41 +0000 Subject: [PATCH 3/3] Further changes to escape dollar and new line characters in the properties and the updated test script for the fix. --- src/spec-node/dockerCompose.ts | 2 +- src/test/cli.up.test.ts | 34 +++++++++++++++++++ .../.devcontainer/devcontainer.json | 15 ++++++++ .../.devcontainer/docker-compose.yml | 12 +++++++ 4 files changed, 62 insertions(+), 1 deletion(-) create mode 100644 src/test/configs/image-containerEnv-issue/.devcontainer/devcontainer.json create mode 100644 src/test/configs/image-containerEnv-issue/.devcontainer/docker-compose.yml diff --git a/src/spec-node/dockerCompose.ts b/src/spec-node/dockerCompose.ts index e36e6d817..22abb04f6 100644 --- a/src/spec-node/dockerCompose.ts +++ b/src/spec-node/dockerCompose.ts @@ -557,7 +557,7 @@ while sleep 1 & wait $$!; do :; done", "-"${userEntrypoint.map(a => `, ${JSON.st init: true` : ''}${user ? ` user: ${user}` : ''}${Object.keys(env).length ? ` environment:${Object.keys(env).map(key => ` - - '${key}=${env[key].replace(/'/g, '\'\'')}'`).join('')}` : ''}${mergedConfig.privileged ? ` + - '${key}=${env[key].replace(/\n/g, '\\n').replace(/\$/g, '$$$$').replace(/'/g, '\'\'')}'`).join('')}` : ''}${mergedConfig.privileged ? ` privileged: true` : ''}${capAdd.length ? ` cap_add:${capAdd.map(cap => ` - ${cap}`).join('')}` : ''}${securityOpts.length ? ` diff --git a/src/test/cli.up.test.ts b/src/test/cli.up.test.ts index ad73b0fcd..94515e89a 100644 --- a/src/test/cli.up.test.ts +++ b/src/test/cli.up.test.ts @@ -266,6 +266,40 @@ describe('Dev Containers CLI', function () { await shellExec(`docker rm -f ${containerId}`); }); + it('should follow the correct merge logic for containerEnv using docker compose', async () => { + const res = await shellExec(`${cli} up --workspace-folder ${__dirname}/configs/image-containerEnv-issue`); + const response = JSON.parse(res.stdout); + assert.equal(response.outcome, 'success'); + const containerId: string = response.containerId; + assert.ok(containerId, 'Container id not found.'); + + const somePath = await shellExec(`docker exec ${containerId} bash -c 'echo -n $SOME_PATH'`); + assert.equal('/tmp/path/doc-ver/loc', somePath.stdout); + + const envWithSpaces = await shellExec(`docker exec ${containerId} bash -c 'echo -n $VAR_WITH_SPACES'`); + assert.equal('value with spaces', envWithSpaces.stdout); + + const evalEnvWithCommand = await shellExec(`docker exec ${containerId} bash -c 'eval $ENV_WITH_COMMAND'`); + assert.equal('Hello, World!', evalEnvWithCommand.stdout); + + const envWithTestMessage = await shellExec(`docker exec ${containerId} bash -c 'echo -n $Test_Message'`); + assert.equal('H"\\n\\ne"\'\'\'llo M:;a/t?h&^iKa%#@!``ni,sk_a-', envWithTestMessage.stdout); + + const envWithFormat = await shellExec(`docker exec ${containerId} bash -c 'echo -n $ROSCONSOLE_FORMAT'`); + assert.equal('[$${severity}] [$${walltime:%Y-%m-%d %H:%M:%S}] [$${node}]: $${message}', envWithFormat.stdout); + + const envWithDoubleQuote = await shellExec(`docker exec ${containerId} bash -c 'echo -n $VAR_WITH_QUOTES_WE_WANT_TO_KEEP'`); + assert.equal('value with \"quotes\" we want to keep', envWithDoubleQuote.stdout); + + const envWithDollar = await shellExec(`docker exec ${containerId} bash -c 'echo -n $VAR_WITH_DOLLAR_SIGN'`); + assert.equal('value with $dollar sign', envWithDollar.stdout); + + const envWithBackSlash = await shellExec(`docker exec ${containerId} bash -c 'echo -n $VAR_WITH_BACK_SLASH'`); + assert.equal('value with \\back slash', envWithBackSlash.stdout); + + await shellExec(`docker rm -f ${containerId}`); + }); + it('should run with config in subfolder', async () => { const upRes = await shellExec(`${cli} up --workspace-folder ${__dirname}/configs/dockerfile-without-features --config ${__dirname}/configs/dockerfile-without-features/.devcontainer/subfolder/devcontainer.json`); const response = JSON.parse(upRes.stdout); diff --git a/src/test/configs/image-containerEnv-issue/.devcontainer/devcontainer.json b/src/test/configs/image-containerEnv-issue/.devcontainer/devcontainer.json new file mode 100644 index 000000000..fd40b349c --- /dev/null +++ b/src/test/configs/image-containerEnv-issue/.devcontainer/devcontainer.json @@ -0,0 +1,15 @@ +{ + "dockerComposeFile": "docker-compose.yml", + "service": "devcontainerissues", + "workspaceFolder": "/workspaces/cli", + "containerEnv": { + "SOME_PATH": "/tmp/path/doc-ver/loc", + "Test_Message": "H\"\n\ne\"'''llo M:;a/t?h&^iKa%#@!``ni,sk_a-", + "ROSCONSOLE_FORMAT": "[$${severity}] [$${walltime:%Y-%m-%d %H:%M:%S}] [$${node}]: $${message}", + "VAR_WITH_SPACES": "value with spaces", + "VAR_WITH_QUOTES_WE_WANT_TO_KEEP": "value with \"quotes\" we want to keep", + "VAR_WITH_DOLLAR_SIGN": "value with $dollar sign", + "VAR_WITH_BACK_SLASH": "value with \\back slash", + "ENV_WITH_COMMAND": "bash -c 'echo -n \"Hello, World!\"'" + } +} diff --git a/src/test/configs/image-containerEnv-issue/.devcontainer/docker-compose.yml b/src/test/configs/image-containerEnv-issue/.devcontainer/docker-compose.yml new file mode 100644 index 000000000..c59746dcd --- /dev/null +++ b/src/test/configs/image-containerEnv-issue/.devcontainer/docker-compose.yml @@ -0,0 +1,12 @@ +services: + devcontainerissues: + # set this to the premade image generated by running the 'original_container' vsc-original-container-c521b00ea40fee92e585b105d9e4f1699ad8216f18220c1b914451f2eafef3b4 + image: mcr.microsoft.com/devcontainers/javascript-node:1-22-bookworm + volumes: + - ../..:/workspaces:cached + environment: + FOO: b"\n\ta"r + Test_Message: "Hello Max" + ROSCONSOLE_FORMAT: "Ros from compose" + command: sleep infinity +