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

[Kaniska (Kaniska244)]

What this PR does

Related #1229. Fixes the InvalidDefaultArgInFrom build warning emitted by Docker/BuildKit when building dev containers. The warning is raised whenever a FROM instruction references a build ARG that has no in-scope default value at parse time.

 1 warning found (use docker --debug to expand):
 - InvalidDefaultArgInFrom: Default value for ARG $BASE_IMAGE results in empty or invalid base image name (line 2)
[303440 ms] Start: Run: docker events --format {{json .}} --filter event=start
[303442 ms] Start: Starting container

This warning does not come from the user's project Dockerfile — it originates from the internal Dockerfiles the CLI generates (and the static updateUID.Dockerfile), where FROM referenced _DEV_CONTAINERS_BASE_IMAGE / BASE_IMAGE without a declared default.

Root cause

Two patterns were triggering the linter:

1. No default on the ARG — scripts/updateUID.Dockerfile declared ARG BASE_IMAGE (no default) and then used FROM $BASE_IMAGE.
2. Shell-style fallback that the linter doesn't credit — the generated feature Dockerfiles used FROM ${_DEV_CONTAINERS_BASE_IMAGE:-scratch}. The ${VAR:-scratch} form is a runtime shell expansion; it is not recognized as an ARG default by the static linter, so the warning persisted even when a global ARG ... = placeholder was declared.

The fix

Declare the base-image ARG with a real default (scratch for the generated files, placeholder for the UID script) immediately before it's used, and reference it plainly as $VAR — the form the linter recognizes as having a valid default.

• src/spec-configuration/containerFeaturesConfiguration.ts — add ARG _DEV_CONTAINERS_BASE_IMAGE=scratch before the FROM stages in the generated feature base Dockerfile (the FROM lines already reference the ARG directly).
• src/spec-node/containerFeatures.ts — change the prefix ARG default from placeholder to scratch in both getImageBuildOptions (no-features path) and getFeaturesBuildOptions (features path).
• scripts/updateUID.Dockerfile — change ARG BASE_IMAGE → ARG BASE_IMAGE=placeholder.

Behavior is unchanged at build time

The CLI always passes the real image via --build-arg (_DEV_CONTAINERS_BASE_IMAGE=<image>, and BASE_IMAGE=<image> for the UID Dockerfile), so the scratch / placeholder defaults only ever exist to satisfy the static linter and never affect the actual build.

Tests

Added a daemon-free unit test suite in src/test/container-features/generateFeaturesConfig.test.ts (validate generated Dockerfiles avoid InvalidDefaultArgInFrom) that:

• Introduces a findFromArgsWithoutDefault() helper mimicking the InvalidDefaultArgInFrom rule (flags any ARG used in a FROM without a default declared before it).
• Asserts the generated feature base Dockerfile declares _DEV_CONTAINERS_BASE_IMAGE with a default, references it directly, and does not use the ${VAR:-default} shell fallback.
• Asserts scripts/updateUID.Dockerfile declares BASE_IMAGE with a default before FROM.

These run without Docker, so they execute in CI as part of npm test.

Files changed

File	Change
src/spec-configuration/containerFeaturesConfiguration.ts	Declare ARG _DEV_CONTAINERS_BASE_IMAGE=scratch before FROM
src/spec-node/containerFeatures.ts	ARG default placeholder → scratch in both build-option paths
scripts/updateUID.Dockerfile	ARG BASE_IMAGE → ARG BASE_IMAGE=placeholder
src/test/container-features/generateFeaturesConfig.test.ts	Add InvalidDefaultArgInFrom regression tests

Notes / follow-ups

• The getImageBuildOptions no-features path is covered indirectly (its ARG default was corrected), but its Dockerfile string is built inline and isn't directly unit-tested. A small follow-up could extract that string into an exported helper to unit-test it the same way.
• .devcontainer/devcontainer-lock.json shows a trailing-newline-only change; consider reverting that if it's unintentional.

[Vatsal Gupta (gvatsal60)]

Hi Kaniska (@Kaniska244),
Would it be possible to merge this PR when you get a chance?

The current behaviour is breaking existing workflows.

[Copilot]

Pull request overview

This PR addresses Docker/BuildKit’s InvalidDefaultArgInFrom warning by ensuring Dockerfiles (generated by the CLI and scripts/updateUID.Dockerfile) declare a default value for any ARG referenced by a FROM, using syntax the linter recognizes.

Changes:

• Set _DEV_CONTAINERS_BASE_IMAGE default to scratch in generated Dockerfile prefixes and in the feature base Dockerfile template.
• Set BASE_IMAGE=placeholder in scripts/updateUID.Dockerfile to avoid FROM $BASE_IMAGE default warnings.
• Add unit tests to regression-check generated Dockerfiles for ARG defaults before FROM, plus a helper to detect violations.

Show a summary per file

File	Description
src/test/testUtils.ts	Adds a helper to detect FROM-referenced args lacking defaults (used by new tests).
src/test/container-features/generateFeaturesConfig.test.ts	Adds regression tests ensuring generated Dockerfiles avoid InvalidDefaultArgInFrom.
src/spec-node/containerFeatures.ts	Changes generated Dockerfile prefix default _DEV_CONTAINERS_BASE_IMAGE from placeholder to scratch.
src/spec-configuration/containerFeaturesConfiguration.ts	Adds ARG _DEV_CONTAINERS_BASE_IMAGE=scratch before template FROM stages.
scripts/updateUID.Dockerfile	Adds default BASE_IMAGE=placeholder to satisfy Dockerfile parsing/linting.
.devcontainer/devcontainer-lock.json	Trailing newline / formatting-only change.

Review details

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

• Files reviewed: 5/6 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

Review details

Suppressed comments (2)

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

• This test calls getContainerFeaturesBaseDockerFile() without applying the #{nonBuildKitFeatureContentFallback} replacement, so it doesn’t exercise the non-BuildKit shape where an extra leading FROM ... as dev_containers_feature_content_source is inserted (the case most likely to regress InvalidDefaultArgInFrom). Consider replacing the placeholder in the test before running findFromArgsWithoutDefault().

        const dockerfile = getContainerFeaturesBaseDockerFile('/tmp/build-features');

src/spec-node/containerFeatures.ts:278

• getContainerFeaturesBaseDockerFile() now emits ARG _DEV_CONTAINERS_BASE_IMAGE=scratch, but getFeaturesBuildOptions() also prepends the same ARG via dockerfilePrefixContent. This results in duplicate ARG declarations in the generated Dockerfile (one in the preamble, one before the first stage), which is unnecessary and makes the generated Dockerfile harder to reason about. After ensuring the base Dockerfile declares the ARG before any FROM, you can drop the duplicate from the prefix content here.

        syntax ? `# syntax=${syntax}` : ''}
ARG _DEV_CONTAINERS_BASE_IMAGE=scratch
`;

• Files reviewed: 5/6 changed files
• Comments generated: 1
• Review effort level: Lite

[Kaniska (Kaniska244)]

Hi Abdurrahmaan Iqbal (@abdurriq)

Kindly note that its premise ignores the global ARG inserted later here and evaluates an intermediate fragment that is not independently consumed. Moving the declaration is perhaps harmless, but it is unnecessary and would encode the wrong abstraction boundary in the test.