fix: set default base image in Dockerfiles and improve syntax handling (#1271)

* fix: set default base image in Dockerfiles and improve syntax handling

* Retrigger test

* Addressing review comments.

* Retrigger tests

* Fixing failing tests due to change in terraform feature to add hard dependency of github-cli feature.

* Removing unrelated test fixes

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

---------

Co-authored-by: Abdurrahmaan Iqbal <abdurriq@github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Author: 3 people

.devcontainer/devcontainer-lock.json

@@ -6,4 +6,4 @@
       "integrity": "sha256:ce078b7bf7d9ef3bcb9813b32103795d8d72172446890b64772cbe1dec6baafd"
     }
   }
-}
+}

scripts/updateUID.Dockerfile

@@ -1,6 +1,6 @@
 #  Copyright (c) Microsoft Corporation. All rights reserved.
 #  Licensed under the MIT License. See License.txt in the project root for license information.
-ARG BASE_IMAGE
+ARG BASE_IMAGE=placeholder
 FROM $BASE_IMAGE
 
 USER root

src/spec-configuration/containerFeaturesConfiguration.ts

@@ -203,6 +203,8 @@ export function getContainerFeaturesBaseDockerFile(contentSourceRootPath: string
 
 #{nonBuildKitFeatureContentFallback}
 
+ARG _DEV_CONTAINERS_BASE_IMAGE=scratch
+
 FROM $_DEV_CONTAINERS_BASE_IMAGE AS dev_containers_feature_content_normalize
 USER root
 COPY --from=dev_containers_feature_content_source ${path.posix.join(contentSourceRootPath, 'devcontainer-features.builtin.env')} /tmp/build-features/

src/spec-node/containerFeatures.ts

@@ -206,7 +206,7 @@ ${getDevcontainerMetadataLabel(getDevcontainerMetadata(imageBuildInfo.metadata,
 `,
         overrideTarget: 'dev_containers_target_stage',
         dockerfilePrefixContent: `${syntax ? `# syntax=${syntax}` : ''}
-    ARG _DEV_CONTAINERS_BASE_IMAGE=placeholder
+    ARG _DEV_CONTAINERS_BASE_IMAGE=scratch
 `,
         buildArgs: {
             _DEV_CONTAINERS_BASE_IMAGE: baseName,
@@ -274,7 +274,7 @@ async function getFeaturesBuildOptions(params: DockerResolverParameters, devCont
         skipDefaultSyntax ? (syntax ? `# syntax=${syntax}` : '') :
         useBuildKitBuildContexts && !(imageBuildInfo.dockerfile && supportsBuildContexts(imageBuildInfo.dockerfile)) ? '# syntax=docker/dockerfile:1.4' :
         syntax ? `# syntax=${syntax}` : ''}
-ARG _DEV_CONTAINERS_BASE_IMAGE=placeholder
+ARG _DEV_CONTAINERS_BASE_IMAGE=scratch
 `;
 
 	// Build devcontainer-features.env and devcontainer-features-install.sh file(s) for each features source folder

src/test/container-features/generateFeaturesConfig.test.ts

@@ -1,23 +1,23 @@
 import { assert } from 'chai';
-import { generateFeaturesConfig, getFeatureLayers, FeatureSet } from '../../spec-configuration/containerFeaturesConfiguration';
+import { generateFeaturesConfig, getFeatureLayers, getContainerFeaturesBaseDockerFile, FeatureSet } from '../../spec-configuration/containerFeaturesConfiguration';
 import { createPlainLog, LogLevel, makeLog } from '../../spec-utils/log';
 import * as path from 'path';
 import * as process from 'process';
 import * as os from 'os';
 import * as crypto from 'crypto';
-import { mkdirpLocal } from '../../spec-utils/pfs';
+import { mkdirpLocal, readLocalFile } from '../../spec-utils/pfs';
 import { DevContainerConfig } from '../../spec-configuration/configuration';
 import { URI } from 'vscode-uri';
 import { getLocalCacheFolder } from '../../spec-node/utils';
-import { shellExec } from '../testUtils';
+import { findFromArgsWithoutDefault, shellExec } from '../testUtils';
 import { getEntPasswdShellCommand } from '../../spec-common/commonUtils';
 
 export const output = makeLog(createPlainLog(text => process.stdout.write(text), () => LogLevel.Trace));
 
-// Test fetching/generating the devcontainer-features.json config
+// Testing fetching/generating the devcontainer-features.json config
 describe('validate generateFeaturesConfig()', function () {
 
-    // Setup
+    // Setup for tests
     const env = { 'SOME_KEY': 'SOME_VAL' };
     const platform = process.platform;
 	const cacheFolder = path.join(os.tmpdir(), `devcontainercli-test-${crypto.randomUUID()}`);
@@ -135,4 +135,25 @@ RUN chmod -R 0755 /tmp/dev-container-features/hello_1 \\
         const javaSettings = java?.features[0]?.customizations?.vscode?.settings;
         assert.isObject(javaSettings);
     });
+});
+
+describe('validate generated Dockerfiles avoid InvalidDefaultArgInFrom', function () {
+
+    it('feature base Dockerfile declares _DEV_CONTAINERS_BASE_IMAGE with a default before FROM', function () {
+        const dockerfile = getContainerFeaturesBaseDockerFile('/tmp/build-features');
+        assert.match(dockerfile, /ARG _DEV_CONTAINERS_BASE_IMAGE=\S+/, 'ARG should have a default value');
+        assert.match(dockerfile, /FROM \$_DEV_CONTAINERS_BASE_IMAGE\b/, 'FROM should reference the ARG directly');
+        assert.notMatch(dockerfile, /FROM \$\{_DEV_CONTAINERS_BASE_IMAGE:-/, 'should not rely on ${VAR:-default} shell fallback');
+        assert.deepStrictEqual(findFromArgsWithoutDefault(dockerfile), []);
+    });
+
+    it('validate that updateUID.Dockerfile declares BASE_IMAGE with a default before FROM', async function () {
+        const content = (await readLocalFile('scripts/updateUID.Dockerfile')).toString();
+        assert.match(content, /ARG BASE_IMAGE=\S+/, 'BASE_IMAGE ARG should have a default value');
+        assert.deepStrictEqual(
+            findFromArgsWithoutDefault(content),
+            [],
+            'updateUID.Dockerfile FROM references an ARG without a default'
+        );
+    });
 });

src/test/testUtils.ts

@@ -126,6 +126,36 @@ export async function pathExists(cli: string, workspaceFolder: string, location:
         return false;
     }
 }
+
+export function findFromArgsWithoutDefault(dockerfile: string): string[] {
+    const preambleArgsWithDefault = new Set<string>();
+    const offenders: string[] = [];
+    let beforeFirstFrom = true;
+
+    for (const rawLine of dockerfile.split('\n')) {
+        const line = rawLine.trim();
+
+        const argWithDefault = /^ARG\s+([A-Za-z0-9_]+)\s*=\s*\S+/.exec(line);
+        if (argWithDefault) {
+            if (beforeFirstFrom) {
+                preambleArgsWithDefault.add(argWithDefault[1]);
+            }
+            continue;
+        }
+
+        const fromMatch = /^FROM(?:\s+--platform=\S+)?\s+\$\{?([A-Za-z0-9_]+)/i.exec(line);
+        if (fromMatch && !preambleArgsWithDefault.has(fromMatch[1])) {
+            offenders.push(fromMatch[1]);
+        }
+
+        if (/^FROM\b/i.test(line)) {
+            beforeFirstFrom = false;
+        }
+    }
+
+    return offenders;
+}
+
 export async function commandMarkerTests(cli: string, workspaceFolder: string, expected: { postCreate: boolean; postStart: boolean; postAttach: boolean }, message: string) {
     const actual = {
         postCreate: await pathExists(cli, workspaceFolder, '/tmp/postCreateCommand.testmarker'),