Skip to content
Closed
Show file tree
Hide file tree
Changes from 9 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
15 changes: 14 additions & 1 deletion .github/workflows/dev-containers.yml
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,19 @@ jobs:
node-version: '18.x'
registry-url: 'https://npm.pkg.github.com'
scope: '@microsoft'
- name: Docker Version (before update)
run: docker version
- name: Update Docker
run: |
# Add Docker's official APT repository
sudo install -m 0755 -d /etc/apt/keyrings
sudo curl -fsSL https://download.docker.com/linux/ubuntu/gpg -o /etc/apt/keyrings/docker.asc
sudo chmod a+r /etc/apt/keyrings/docker.asc
echo "deb [arch=$(dpkg --print-architecture) signed-by=/etc/apt/keyrings/docker.asc] https://download.docker.com/linux/ubuntu $(. /etc/os-release && echo "$VERSION_CODENAME") stable" | sudo tee /etc/apt/sources.list.d/docker.list > /dev/null
sudo apt-get update
sudo apt-get install -y docker-ce docker-ce-cli containerd.io docker-buildx-plugin
- name: Docker Version (after update)
run: docker version
- name: Tools Info
run: |
docker info
Expand All @@ -84,7 +97,7 @@ jobs:
- name: Package
run: yarn package
- name: Run Tests
run: yarn test-matrix --forbid-only ${{ matrix.mocha-args }}
run: yarn test-matrix ${{ matrix.mocha-args }}
env:
CI: true

Expand Down
86 changes: 86 additions & 0 deletions .github/workflows/docker-platform-bug.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
name: Docker Platform Bug Repro

on:
push:
branches:
- 'chrmarti/**'

jobs:
repro:
name: Platform Bug Repro
runs-on: ubuntu-latest
steps:
- name: Update Docker
run: |
sudo install -m 0755 -d /etc/apt/keyrings
sudo curl -fsSL https://download.docker.com/linux/ubuntu/gpg -o /etc/apt/keyrings/docker.asc
sudo chmod a+r /etc/apt/keyrings/docker.asc
echo "deb [arch=$(dpkg --print-architecture) signed-by=/etc/apt/keyrings/docker.asc] https://download.docker.com/linux/ubuntu $(. /etc/os-release && echo "$VERSION_CODENAME") stable" | sudo tee /etc/apt/sources.list.d/docker.list > /dev/null
sudo apt-get update
sudo apt-get install -y docker-ce docker-ce-cli containerd.io docker-buildx-plugin
- name: Setup
run: |
docker version
docker info | grep -E "Storage Driver|driver-type"
docker run --privileged --rm tonistiigi/binfmt --install all
- name: Build with platform in Dockerfile only
run: |
dir=$(mktemp -d)
echo 'FROM --platform=linux/arm64 debian:latest' > "$dir/Dockerfile"
docker buildx build --load -t test-arm64-in-dockerfile "$dir"
- name: Inspect manifest list (Dockerfile platform)
run: |
docker image inspect test-arm64-in-dockerfile --format '{{.Architecture}}'
digest=$(docker image inspect test-arm64-in-dockerfile --format '{{.Id}}')
docker save test-arm64-in-dockerfile | tar -xO blobs/sha256/${digest#sha256:} | python3 -c "
import sys, json
data = json.load(sys.stdin)
if 'manifests' in data:
print('Manifest list found:')
for m in data['manifests']:
p = m.get('platform', {})
t = m.get('annotations', {}).get('vnd.docker.reference.type', 'image')
print(f' type={t} arch={p.get(\"architecture\")} os={p.get(\"os\")} variant={p.get(\"variant\", \"\")}')
else:
print('No manifest list (single manifest)')
"
- name: Build using that image as base with --platform linux/arm64
run: |
dir=$(mktemp -d)
echo 'FROM test-arm64-in-dockerfile' > "$dir/Dockerfile"
echo "Expecting this to fail due to manifest list platform mismatch..."
if docker build --platform linux/arm64 -t test-arm64-rebuild "$dir" 2>&1; then
echo "BUILD SUCCEEDED (bug may be fixed)"
else
echo "BUILD FAILED (bug confirmed)"
fi
- name: Build with --platform on CLI
run: |
dir=$(mktemp -d)
echo 'FROM debian:latest' > "$dir/Dockerfile"
docker buildx build --load --platform linux/arm64 -t test-arm64-on-cli "$dir"
- name: Inspect manifest list (CLI platform)
run: |
docker image inspect test-arm64-on-cli --format '{{.Architecture}}'
digest=$(docker image inspect test-arm64-on-cli --format '{{.Id}}')
docker save test-arm64-on-cli | tar -xO blobs/sha256/${digest#sha256:} | python3 -c "
import sys, json
data = json.load(sys.stdin)
if 'manifests' in data:
print('Manifest list found:')
for m in data['manifests']:
p = m.get('platform', {})
t = m.get('annotations', {}).get('vnd.docker.reference.type', 'image')
print(f' type={t} arch={p.get(\"architecture\")} os={p.get(\"os\")} variant={p.get(\"variant\", \"\")}')
else:
print('No manifest list (single manifest)')
"
- name: Build using CLI-platform image with --platform linux/arm64
run: |
dir=$(mktemp -d)
echo 'FROM test-arm64-on-cli' > "$dir/Dockerfile"
if docker build --platform linux/arm64 -t test-arm64-on-cli-rebuild "$dir" 2>&1; then
echo "BUILD SUCCEEDED (expected)"
else
echo "BUILD FAILED (unexpected)"
fi
Comment on lines +10 to +87

Check warning

Code scanning / CodeQL

Workflow does not contain permissions Medium

Actions job or workflow does not limit the permissions of the GITHUB_TOKEN. Consider setting an explicit permissions block, using the following as a minimal starting point: {}

Copilot Autofix

AI 2 months ago

In general, you fix this kind of issue by adding an explicit permissions: block either at the workflow root (to apply to all jobs) or at the individual job level, granting only the specific scopes required. If a job does not need any GITHUB_TOKEN access, you can safely set permissions: { contents: read } or even permissions: {} at the job level to fully disable the token.

For this workflow, the repro job only installs Docker and runs local docker and python3 commands, and does not interact with the GitHub API or repository contents via GITHUB_TOKEN. The safest and clearest fix, without changing existing functionality, is to add a minimal permissions block at the job level. To be maximally restrictive and align with least privilege, we can disable the token entirely for this job using permissions: {} under jobs.repro. This documents that the job does not need any token capabilities and ensures that even if repository defaults change in the future, this job will not receive unnecessary permissions.

Concretely: in .github/workflows/docker-platform-bug.yml, under jobs: repro: name: Platform Bug Repro, insert a permissions: {} line (with correct YAML indentation) between the name: and runs-on: lines. No imports or additional methods are needed, since this is purely a YAML configuration change.

Suggested changeset 1
.github/workflows/docker-platform-bug.yml

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/.github/workflows/docker-platform-bug.yml b/.github/workflows/docker-platform-bug.yml
--- a/.github/workflows/docker-platform-bug.yml
+++ b/.github/workflows/docker-platform-bug.yml
@@ -8,6 +8,7 @@
 jobs:
   repro:
     name: Platform Bug Repro
+    permissions: {}
     runs-on: ubuntu-latest
     steps:
     - name: Update Docker
EOF
@@ -8,6 +8,7 @@
jobs:
repro:
name: Platform Bug Repro
permissions: {}
runs-on: ubuntu-latest
steps:
- name: Update Docker
Copilot is powered by AI and may make mistakes. Always verify output.
43 changes: 40 additions & 3 deletions src/spec-node/containerFeatures.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import * as path from 'path';

import { DevContainerConfig } from '../spec-configuration/configuration';
import { dockerCLI, dockerPtyCLI, ImageDetails, toExecParameters, toPtyExecParameters } from '../spec-shutdown/dockerUtils';
import { LogLevel, makeLog } from '../spec-utils/log';
import { LogLevel, makeLog, nullLog } from '../spec-utils/log';
import { FeaturesConfig, getContainerFeaturesBaseDockerFile, getFeatureInstallWrapperScript, getFeatureLayers, getFeatureMainValue, getFeatureValueObject, generateFeaturesConfig, Feature, generateContainerEnvs } from '../spec-configuration/containerFeaturesConfiguration';
import { readLocalFile } from '../spec-utils/pfs';
import { includeAllConfiguredFeatures } from '../spec-utils/product';
Expand Down Expand Up @@ -420,12 +420,13 @@ function getFeatureEnvVariables(f: Feature) {

export async function getRemoteUserUIDUpdateDetails(params: DockerResolverParameters, mergedConfig: MergedDevContainerConfig, imageName: string, imageDetails: () => Promise<ImageDetails>, runArgsUser: string | undefined) {
const { common } = params;
const { cliHost } = common;
const { cliHost, output } = common;
const { updateRemoteUserUID } = mergedConfig;
if (params.updateRemoteUserUIDDefault === 'never' || !(typeof updateRemoteUserUID === 'boolean' ? updateRemoteUserUID : params.updateRemoteUserUIDDefault === 'on') || !(cliHost.platform === 'linux' || params.updateRemoteUserUIDOnMacOS && cliHost.platform === 'darwin')) {
return null;
}
const details = await imageDetails();
output.write(`updateUID: image=${imageName} Os=${details.Os} Architecture=${details.Architecture} Variant=${details.Variant || '(none)'}`, LogLevel.Info);
const imageUser = details.Config.User || 'root';
const remoteUser = mergedConfig.remoteUser || runArgsUser || imageUser;
if (remoteUser === 'root' || /^\d+$/.test(remoteUser)) {
Expand All @@ -434,11 +435,13 @@ export async function getRemoteUserUIDUpdateDetails(params: DockerResolverParame
const folderImageName = getFolderImageName(common);
const fixedImageName = `${imageName.startsWith(folderImageName) ? imageName : folderImageName}-uid`;

const platform = [details.Os, details.Architecture, details.Variant].filter(Boolean).join('/');
output.write(`updateUID: remoteUser=${remoteUser} imageUser=${imageUser} platform=${platform}`, LogLevel.Info);
return {
imageName: fixedImageName,
remoteUser,
imageUser,
platform: [details.Os, details.Architecture, details.Variant].filter(Boolean).join('/')
platform,
};
}

Expand All @@ -451,6 +454,39 @@ export async function updateRemoteUserUID(params: DockerResolverParameters, merg
return imageName;
}
const { imageName: fixedImageName, remoteUser, imageUser, platform } = updateDetails;
const { output } = common;

try {
const infoResult = await dockerCLI(params, 'info', '--format', '{{.Driver}} / containerd: {{.DriverStatus}}');
output.write(`updateUID: docker info: ${infoResult.stdout.toString().trim()}`, LogLevel.Info);
} catch (err) {
output.write(`updateUID: docker info failed: ${err}`, LogLevel.Warning);
}
try {
const inspectResult = await dockerCLI(params, 'inspect', '--type', 'image', imageName);
const inspectJson = inspectResult.stdout.toString().trim();
output.write(`updateUID: docker inspect ${imageName}: ${inspectJson}`, LogLevel.Info);
// Extract the OCI index from docker save to see manifest list platform annotations
try {
const parsed = JSON.parse(inspectJson);
const digest = parsed[0]?.Descriptor?.digest;
if (digest) {
const hash = digest.replace('sha256:', '').replace(':', '/');
// docker save outputs OCI layout; extract the manifest list blob by digest
const saveResult = await runCommandNoPty({
exec: common.cliHost.exec,
cmd: '/bin/sh',
args: ['-c', `docker save ${imageName} | tar -xO blobs/sha256/${hash} 2>/dev/null || docker save ${imageName} | tar -xO index.json 2>/dev/null`],
output: nullLog
});
output.write(`updateUID: manifest list for ${imageName}: ${saveResult.stdout.toString().trim()}`, LogLevel.Info);
}
} catch (blobErr) {
output.write(`updateUID: reading manifest list failed: ${blobErr instanceof Error ? blobErr.message : JSON.stringify(blobErr)}`, LogLevel.Warning);
}
} catch (err) {
output.write(`updateUID: docker inspect failed: ${err}`, LogLevel.Warning);
}

const dockerfileName = 'updateUID.Dockerfile';
const srcDockerfile = path.join(common.extensionPath, 'scripts', dockerfileName);
Expand All @@ -474,6 +510,7 @@ export async function updateRemoteUserUID(params: DockerResolverParameters, merg
'--build-arg', `IMAGE_USER=${imageUser}`,
emptyFolder,
];
output.write(`updateUID: docker ${args.join(' ')}`, LogLevel.Info);
if (params.isTTY) {
await dockerPtyCLI(params, ...args);
} else {
Expand Down
2 changes: 1 addition & 1 deletion src/test/testUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ export async function shellPtyExec(command: string, options: { stdin?: string }
}

export async function devContainerUp(cli: string, workspaceFolder: string, options?: { cwd?: string; useBuildKit?: boolean; userDataFolder?: string; logLevel?: string; extraArgs?: string; prefix?: string; env?: NodeJS.ProcessEnv }): Promise<UpResult> {
const buildkitOption = (options?.useBuildKit ?? false) ? '' : ' --buildkit=never';
const buildkitOption = (options?.useBuildKit ?? true) ? '' : ' --buildkit=never';
const userDataFolderOption = (options?.userDataFolder ?? false) ? ` --user-data-folder=${options?.userDataFolder}` : '';
const logLevelOption = (options?.logLevel ?? false) ? ` --log-level ${options?.logLevel}` : '';
const extraArgs = (options?.extraArgs ?? false) ? ` ${options?.extraArgs}` : '';
Expand Down
2 changes: 1 addition & 1 deletion src/test/updateUID.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { devContainerDown, devContainerUp, shellExec } from './testUtils';

const pkg = require('../../package.json');

(process.platform === 'linux' ? describe : describe.skip)('Dev Containers CLI', function () {
(process.platform === 'linux' ? describe.only : describe.skip)('Dev Containers CLI', function () {
this.timeout('120s');

const tmp = path.relative(process.cwd(), path.join(__dirname, 'tmp'));
Expand Down
Loading